On 10/04/2017 03:05 AM, Martin Liška wrote:
Hello.
Following patch adds support for optimizing out unnecessary UBSAN_PTR checks.
It handles separately positive and negative offsets, zero offset is ignored.
Apart from that, we utilize get_inner_reference for local and global variables,
that also helps to reduce some.
Some numbers:
1) postgres:
bloaty /tmp/after2 -- /tmp/before2
VM SIZE FILE SIZE
++++++++++++++ GROWING ++++++++++++++
[ = ] 0 .debug_abbrev +1.84Ki +0.3%
-------------- SHRINKING --------------
-30.3% -3.98Mi .text -3.98Mi -30.3%
[ = ] 0 .debug_info -3.69Mi -17.2%
[ = ] 0 .debug_loc -2.02Mi -13.4%
-43.1% -1.37Mi .data -1.37Mi -43.1%
[ = ] 0 .debug_ranges -390Ki -14.3%
[ = ] 0 .debug_line -295Ki -11.6%
-4.0% -26.3Ki .eh_frame -26.3Ki -4.0%
[ = ] 0 [Unmapped] -1.61Ki -62.3%
[ = ] 0 .strtab -1.15Ki -0.4%
[ = ] 0 .symtab -1.08Ki -0.3%
-0.4% -368 .eh_frame_hdr -368 -0.4%
[ = ] 0 .debug_aranges -256 -0.7%
[DEL] -16 [None] 0 [ = ]
-28.0% -5.37Mi TOTAL -11.8Mi -18.8%
This looks like an impressive improvement! FWIW, I've been
meaning to look into similar opportunities mentioned in bug
79265.
Would making use of get_range_info() make sense here to detect
and eliminate even more cases?
Just a few minor mostly stylistic suggestions.
+/* Traits class for tree triplet hash maps below. */
+
+struct sanopt_tree_couple_hash : typed_noop_remove <sanopt_tree_couple>
+{
...
+ static inline bool
+ equal (const sanopt_tree_couple &ref1, const sanopt_tree_couple &ref2)
Member functions defined within the body of the class are implicitly
inline so while not wrong, there is no need to declare them inline
explicitly.
Also, since mark_deleted uses reinterpret_cast (as suggested by
GCC coding conventions) it seems that is_deleted should do the
same for consistency. Alternatively, if there isn't enough
interest/consensus to follow this guideline perhaps it should
be removed from the GCC coding conventions. (Very few GCC code
seems to use reinterpret_cast.)
+/* Return true when pointer PTR for a given OFFSET is already sanitized
+ in a given sanitization context CTX. */
Shouldn't the comment read CUR_OFFSET? I ask because the function
also declares a local variable OFFSET.
+static bool
+has_dominating_ubsan_ptr_check (sanopt_ctx *ctx, tree ptr, tree cur_offset)
+{
...
+ /* We already have recorded a UBSAN_PTR check for this pointer.
Perhaps we
+ can drop this one. But only if this check doesn't specify larger
offset.
+ */
+ tree offset = gimple_call_arg (g, 1);
Martin
PS It seems to me that the test could be enabled for all targets
where UBSan is supported by making use of SIZE_MAX to compute
the values of the constants instead of hardwiring LP64 values.
I noticed the test doesn't exercise member arrays. Are those
not handled by the patch?
Left checks:
261039
Optimized out:
85643
2) tramp3d:
bloaty after -- before
VM SIZE FILE SIZE
++++++++++++++ GROWING ++++++++++++++
+167% +30 [Unmapped] +1.01Ki +39%
-------------- SHRINKING --------------
-58.5% -2.52Mi .text -2.52Mi -58.5%
-64.2% -574Ki .data -574Ki -64.2%
-5.7% -4.27Ki .eh_frame -4.27Ki -5.7%
-6.4% -1.06Ki .gcc_except_table -1.06Ki -6.4%
-7.2% -192 .bss 0 [ = ]
-0.1% -32 .rodata -32 -0.1%
[ = ] 0 .strtab -29 -0.0%
-1.1% -24 .dynsym -24 -1.1%
-1.5% -24 .rela.plt -24 -1.5%
[ = ] 0 .symtab -24 -0.1%
-0.6% -16 .dynstr -16 -0.6%
-1.5% -16 .plt -16 -1.5%
-1.4% -8 .got.plt -8 -1.4%
-0.6% -4 .hash -4 -0.6%
-1.1% -2 .gnu.version -2 -1.1%
-58.0% -3.09Mi TOTAL -3.08Mi -55.0%
Left checks:
31131
Optimized out:
36752
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.
Ready to be installed?
Martin
gcc/ChangeLog:
2017-10-04 Martin Liska <mli...@suse.cz>
* sanopt.c (struct sanopt_tree_couple): New struct.
(struct sanopt_tree_couple_hash): Likewise.
(struct sanopt_ctx): Add ptr_check_map.
(has_dominating_ubsan_ptr_check): New function.
(record_ubsan_ptr_check_stmt): Likewise.
(maybe_optimize_ubsan_ptr_ifn): Likewise.
(sanopt_optimize_walker): Handle IFN_UBSAN_PTR. Dump info
inline and newly print stmts that are left in code stream.
(pass_sanopt::execute): Handle also SANITIZE_POINTER_OVERFLOW.
gcc/testsuite/ChangeLog:
2017-10-04 Martin Liska <mli...@suse.cz>
* c-c++-common/ubsan/ptr-overflow-sanitization-1.c: New test.
---
gcc/sanopt.c | 212 ++++++++++++++++++++-
.../ubsan/ptr-overflow-sanitization-1.c | 38 ++++
2 files changed, 244 insertions(+), 6 deletions(-)
create mode 100644
gcc/testsuite/c-c++-common/ubsan/ptr-overflow-sanitization-1.c
