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_.

>  };
>  
>  /* This controls how ubsan prints types.  Used in ubsan_type_descriptor.  */
> -- 
> 2.43.0
> 

Marek

Reply via email to