On Mon, Sep 22, 2025 at 10:26 AM Marek Polacek <[email protected]> wrote: > > On Sun, Sep 21, 2025 at 06:32:26PM -0700, Andrew Pinski wrote: > > The problem here is sanopt will remove > > USBAN(0B, 3B, 0) > > in some cases even if this was originally from `&a->b` expression. > > > > The fix here is change ckind to include if it was originally > > from an address and mask it off when expanding the code to creating > > the __ubsan_null_data. > > > > Bootstrapped and tested on x86_64-linux-gnu. > > > > PR sanitizer/122013 > > > > gcc/ChangeLog: > > > > * ubsan.cc (ubsan_expand_null_ifn): Mask off the non-ckind part > > of kind. > > (instrument_mem_ref): Add new was_addr argument. Set the bit > > for ckind. > > (instrument_null): Update call to instrument_mem_ref > > if it was an ADDR_EXPR. > > * ubsan.h (enum ubsan_null_ckind): Add USBAN_NULL_CKIND_MASK > > and USBAN_FROM_ADDR. > > > > gcc/testsuite/ChangeLog: > > > > * c-c++-common/ubsan/null-12a.c: New test. > > > > Signed-off-by: Andrew Pinski <[email protected]> > > --- > > gcc/testsuite/c-c++-common/ubsan/null-12a.c | 43 +++++++++++++++++++++ > > gcc/ubsan.cc | 17 +++++--- > > gcc/ubsan.h | 4 +- > > 3 files changed, 58 insertions(+), 6 deletions(-) > > create mode 100644 gcc/testsuite/c-c++-common/ubsan/null-12a.c > > > > diff --git a/gcc/testsuite/c-c++-common/ubsan/null-12a.c > > b/gcc/testsuite/c-c++-common/ubsan/null-12a.c > > new file mode 100644 > > index 00000000000..40327e4ad34 > > --- /dev/null > > +++ b/gcc/testsuite/c-c++-common/ubsan/null-12a.c > > @@ -0,0 +1,43 @@ > > +/* PR sanitizer/80797 */ > > +/* PR sanitizer/122013 */ > > +/* { dg-do run } */ > > +/* { dg-options "-fsanitize=null -fno-sanitize=address" } */ > > + > > +struct S > > +{ > > + int i; > > +}; > > + > > +struct R > > +{ > > + struct T { > > + int i; > > + } *t; > > +} r; > > + > > +int > > +main () > > +{ > > + struct S *s = 0; > > + struct S *s2[1] = { }; > > + > > + int *v1 = &s->i; > > + int *v2 = &(*s).i; > > + int *v3 = &s2[0]->i; > > + int *v4 = &s->i + 1; > > + int *v5 = &r.t->i; > > + > > + asm ("" : : "r" (&v1) : "memory"); > > + asm ("" : : "r" (&v2) : "memory"); > > + asm ("" : : "r" (&v3) : "memory"); > > + asm ("" : : "r" (&v4) : "memory"); > > + asm ("" : : "r" (&v5) : "memory"); > > + > > + return 0; > > +} > > + > > +/* { dg-output "member access within null pointer of type 'struct > > S'\[^\n\r]*(\n|\r\n|\r)" } */ > > +/* { dg-output "\[^\n\r]*member access within null pointer of type 'struct > > S'\[^\n\r]*(\n|\r\n|\r)" } */ > > +/* { dg-output "\[^\n\r]*member access within null pointer of type 'struct > > S'\[^\n\r]*(\n|\r\n|\r)" } */ > > +/* { dg-output "\[^\n\r]*member access within null pointer of type 'struct > > S'\[^\n\r]*(\n|\r\n|\r)" } */ > > +/* { dg-output "\[^\n\r]*member access within null pointer of type 'struct > > T'" } */ > > diff --git a/gcc/ubsan.cc b/gcc/ubsan.cc > > index 6d748258b1e..16cd7bfd3d5 100644 > > --- a/gcc/ubsan.cc > > +++ b/gcc/ubsan.cc > > @@ -912,6 +912,7 @@ ubsan_expand_null_ifn (gimple_stmt_iterator *gsip) > > : BUILT_IN_UBSAN_HANDLE_TYPE_MISMATCH_V1_ABORT; > > tree fn = builtin_decl_implicit (bcode); > > int align_log = tree_log2 (align); > > + int ckind_i = tree_to_uhwi (ckind) & USBAN_NULL_CKIND_MASK; > > tree data > > = ubsan_create_data ("__ubsan_null_data", 1, &loc, > > ubsan_type_descriptor (TREE_TYPE (ckind), > > @@ -919,7 +920,7 @@ ubsan_expand_null_ifn (gimple_stmt_iterator *gsip) > > NULL_TREE, > > build_int_cst (unsigned_char_type_node, > > MAX (align_log, 0)), > > - fold_convert (unsigned_char_type_node, ckind), > > + build_int_cst (unsigned_char_type_node, ckind_i), > > NULL_TREE); > > data = build_fold_addr_expr_loc (loc, data); > > g = gimple_build_call (fn, 2, data, > > @@ -1436,9 +1437,9 @@ ubsan_expand_vptr_ifn (gimple_stmt_iterator *gsip) > > > > static void > > instrument_mem_ref (tree mem, tree base, gimple_stmt_iterator *iter, > > - bool is_lhs) > > + bool is_lhs, bool was_addr) > > { > > - enum ubsan_null_ckind ikind = is_lhs ? UBSAN_STORE_OF : UBSAN_LOAD_OF; > > + ubsan_null_ckind ikind = is_lhs ? UBSAN_STORE_OF : UBSAN_LOAD_OF; > > unsigned int align = 0; > > if (sanitize_flags_p (SANITIZE_ALIGNMENT)) > > { > > @@ -1460,6 +1461,8 @@ instrument_mem_ref (tree mem, tree base, > > gimple_stmt_iterator *iter, > > return; > > if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (base)) && mem != base) > > ikind = UBSAN_MEMBER_ACCESS; > > + if (was_addr) > > + ikind = ubsan_null_ckind(ikind | USBAN_FROM_ADDR); > > tree kind = build_int_cst (build_pointer_type (TREE_TYPE (base)), ikind); > > tree alignt = build_int_cst (pointer_sized_int_node, align); > > gcall *g = gimple_build_call_internal (IFN_UBSAN_NULL, 3, t, kind, > > alignt); > > @@ -1472,14 +1475,18 @@ instrument_mem_ref (tree mem, tree base, > > gimple_stmt_iterator *iter, > > static void > > instrument_null (gimple_stmt_iterator gsi, tree t, bool is_lhs) > > { > > + bool was_addr = false; > > /* Handle also e.g. &s->i. */ > > if (TREE_CODE (t) == ADDR_EXPR) > > - t = TREE_OPERAND (t, 0); > > + { > > + t = TREE_OPERAND (t, 0); > > + was_addr = true; > > + } > > tree base = get_base_address (t); > > if (base != NULL_TREE > > && TREE_CODE (base) == MEM_REF > > && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME) > > - instrument_mem_ref (t, base, &gsi, is_lhs); > > + instrument_mem_ref (t, base, &gsi, is_lhs, was_addr); > > } > > > > /* Instrument pointer arithmetics PTR p+ OFF. */ > > diff --git a/gcc/ubsan.h b/gcc/ubsan.h > > index 3014edebdd7..2ad81280a26 100644 > > --- a/gcc/ubsan.h > > +++ b/gcc/ubsan.h > > @@ -32,7 +32,9 @@ enum ubsan_null_ckind { > > UBSAN_DOWNCAST_POINTER, > > UBSAN_DOWNCAST_REFERENCE, > > UBSAN_UPCAST, > > - UBSAN_CAST_TO_VBASE > > + UBSAN_CAST_TO_VBASE, > > + USBAN_NULL_CKIND_MASK = 0xF, > > + USBAN_FROM_ADDR = 0x10, > > USBAN_? Please rename it to UBSAN_.
I don't know why or how this typo was introduced. Anyways fixed locally and also fixed the commit message up since I had this typo in there too. Thanks, Andrew > > > }; > > > > /* This controls how ubsan prints types. Used in ubsan_type_descriptor. > > */ > > -- > > 2.43.0 > > > > Marek >
