On 10/06/2017 04:18 AM, Martin Liška wrote:
On 10/05/2017 07:06 PM, Martin Sebor wrote:
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.
Hi. Thank you very much for feedback. If you want I can help with the PR?
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.
Done that in v2.
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.)
Likewise.
+/* 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.
Yes, should be fixed.
+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?
I decided to use __PTRDIF__MAX__ as I need signed type. Is it ok?
I think that should work too. ptrdiff_t is usually the same size
as size_t. The two exceptions I could find are VMS and the M32C
target where size_t is always 32-bits wide but ptrdiff_t can be
64-bits. I don't know what that might mean for the sanitizer (or
if it's even supported there).
Martin
Martin
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
