| Submitter | Jakub Jelinek |
|---|---|
| Date | Dec. 5, 2012, 4:12 p.m. |
| Message ID | <20121205161218.GY2315@tucnak.redhat.com> |
| Download | mbox | patch |
| Permalink | /patch/203899/ |
| State | New |
| Headers | show |
Comments
Hi! This patch tries to handle bitfield accesses as if it were writes/reads from the corresponding DECL_BIT_FIELD_REPRESENTATIVE. Richard, does it make sense this way (the code will take ADDR_EXPR of the COMPONENT_REF with the DECL_BIT_FIELD_REPRESENTATIVE and use the size of the representative if it decides to instrument it. Fixes the first 4 asan_test.C failures. Ok for trunk? 2012-12-05 Jakub Jelinek <jakub@redhat.com> * asan.c (instrument_derefs): Handle bitfield COMPONENT_REFs accesses as reads/writes to their DECL_BIT_FIELD_REPRESENTATIVE. Jakub
On Wed, Dec 05, 2012 at 05:10:56PM +0100, Richard Biener wrote: > > This patch tries to handle bitfield accesses as if it were writes/reads > > from the corresponding DECL_BIT_FIELD_REPRESENTATIVE. > > Richard, does it make sense this way (the code will take ADDR_EXPR > > of the COMPONENT_REF with the DECL_BIT_FIELD_REPRESENTATIVE and use > > the size of the representative if it decides to instrument it. > > > > Fixes the first 4 asan_test.C failures. Ok for trunk? > > It makes sense apart from the address-taking - is this done early > enough to not disrupt aliasing / TREE_ADDRESSABLE? Thus, before > gimplifying? It is done far later than that (except for -Og and -O0 right after PRE), but I thought that isn't a big deal, it takes address for everything to be instrumented, and AFAIK tree-ssa-operands.c will take care of calling mark_addressable on it if needed. Asan doesn't actually dereference that pointer ever, all it does is compute from that address the shadow address (addr >> 3) + (1L << 44) or similar, and dereferences that. And for asan, we want to instrument char p[32]; for (int i = 0; i < n; i++) p[i] = 0; even when p isn't TREE_ADDRESSABLE (ditto for bitfields). So, all in all, even if the address is taken, it will never escape because of the asan instrumentation. Doesn't e.g. ivopts also cause non-addressable vars to become addressable, if it replaces ARRAY_REFs with taking address of the array before the loop and using MEM_REF/TARGET_MEM_REF in the loop? Jakub
On Wed, 5 Dec 2012, Jakub Jelinek wrote: > On Wed, Dec 05, 2012 at 05:10:56PM +0100, Richard Biener wrote: > > > This patch tries to handle bitfield accesses as if it were writes/reads > > > from the corresponding DECL_BIT_FIELD_REPRESENTATIVE. > > > Richard, does it make sense this way (the code will take ADDR_EXPR > > > of the COMPONENT_REF with the DECL_BIT_FIELD_REPRESENTATIVE and use > > > the size of the representative if it decides to instrument it. > > > > > > Fixes the first 4 asan_test.C failures. Ok for trunk? > > > > It makes sense apart from the address-taking - is this done early > > enough to not disrupt aliasing / TREE_ADDRESSABLE? Thus, before > > gimplifying? > > It is done far later than that (except for -Og and -O0 right after PRE), > but I thought that isn't a big deal, it takes address for everything > to be instrumented, and AFAIK tree-ssa-operands.c will take care of > calling mark_addressable on it if needed. Asan doesn't actually dereference > that pointer ever, all it does is compute from that address the shadow > address (addr >> 3) + (1L << 44) or similar, and dereferences that. > And for asan, we want to instrument > char p[32]; > for (int i = 0; i < n; i++) > p[i] = 0; > even when p isn't TREE_ADDRESSABLE (ditto for bitfields). So, all in all, > even if the address is taken, it will never escape because of the asan > instrumentation. Ok. > Doesn't e.g. ivopts also cause non-addressable vars to become addressable, > if it replaces ARRAY_REFs with taking address of the array before the loop > and using MEM_REF/TARGET_MEM_REF in the loop? Yes, but it also doesn't make them escape (autopar does, for example, which is why it has to re-set points-to info ...). Richard.
Jakub Jelinek <jakub@redhat.com> writes: > 2012-12-05 Jakub Jelinek <jakub@redhat.com> > > * asan.c (instrument_derefs): Handle bitfield COMPONENT_REFs > accesses as reads/writes to their DECL_BIT_FIELD_REPRESENTATIVE. After Richi's comments in this thread, I guess this is OK to me.
Patch
--- gcc/asan.c.jj 2012-12-05 15:30:56.000000000 +0100 +++ gcc/asan.c 2012-12-05 17:00:56.957641944 +0100 @@ -792,9 +792,6 @@ instrument_derefs (gimple_stmt_iterator || (unsigned HOST_WIDE_INT) size_in_bytes - 1 >= 16) return; - /* For now just avoid instrumenting bit field acceses. - Fixing it is doable, but expected to be messy. */ - HOST_WIDE_INT bitsize, bitpos; tree offset; enum machine_mode mode; @@ -803,7 +800,17 @@ instrument_derefs (gimple_stmt_iterator &mode, &unsignedp, &volatilep, false); if (bitpos % (size_in_bytes * BITS_PER_UNIT) || bitsize != size_in_bytes * BITS_PER_UNIT) - return; + { + if (TREE_CODE (t) == COMPONENT_REF + && DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (t, 1)) != NULL_TREE) + { + tree repr = DECL_BIT_FIELD_REPRESENTATIVE (TREE_OPERAND (t, 1)); + instrument_derefs (iter, build3 (COMPONENT_REF, TREE_TYPE (repr), + TREE_OPERAND (t, 0), repr, + NULL_TREE), location, is_store); + } + return; + } base = build_fold_addr_expr (t); build_check_stmt (location, base, iter, /*before_p=*/true,