public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/111875] New: With -Og ubsan check inserted even though __builtin_assume_aligned guarantees no UB
@ 2023-10-19  8:54 fkastl at suse dot cz
  2023-10-19  9:25 ` [Bug middle-end/111875] " fkastl at suse dot cz
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: fkastl at suse dot cz @ 2023-10-19  8:54 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111875

            Bug ID: 111875
           Summary: With -Og ubsan check inserted even though
                    __builtin_assume_aligned guarantees no UB
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: fkastl at suse dot cz
  Target Milestone: ---
              Host: x86_64-linux
            Target: x86_64-linux

Running

gcc -S -Og -fno-sanitize=null -fsanitize=alignment
gcc/testsuite/c-c++-common/ubsan/align-5.c

produces code with an alignment undefined behavior check.

This is how the testcase looks like:

/* { dg-do compile } */
/* { dg-options "-fno-sanitize=null -fsanitize=alignment -O2" } */
/* Check that when optimizing if we know the alignment is right
   and we are not doing -fsanitize=null instrumentation we don't
   instrument the alignment check.  */

__attribute__((noinline, noclone)) int 
foo (char *p) 
{
  p = (char *) __builtin_assume_aligned (p, __alignof__(int));
  int *q = (int *) p;
  return *q; 
}

/* { dg-final { scan-assembler-not "__ubsan_handle" } } */

Because of __builtin_assume_aligned, the compiler should assume that p will
always have the correct alignment to be cast to int *.

The compiler produces this (with -Og):

        .file   "align-5.c"
        .text
        .globl  foo 
        .type   foo, @function
foo:
.LFB0:
        .cfi_startproc
        pushq   %rbx
        .cfi_def_cfa_offset 16
        .cfi_offset 3, -16 
        movq    %rdi, %rbx
        testb   $3, %dil
        jne     .L4 
.L2:
        movl    (%rbx), %eax
        popq    %rbx
        .cfi_remember_state
        .cfi_def_cfa_offset 8
        ret
.L4:
        .cfi_restore_state
        movq    %rdi, %rsi
        movl    $.Lubsan_data0, %edi
        call    __ubsan_handle_type_mismatch_v1
        jmp     .L2 
        .cfi_endproc
.LFE0:
        .size   foo, .-foo
        .section        .rodata.str1.1,"aMS",@progbits,1
.LC0:
        .string "align-5.c"
        .data
        .align 32
        .type   .Lubsan_data0, @object
        .size   .Lubsan_data0, 32
.Lubsan_data0:
        .quad   .LC0
        .long   12  
        .long   10  
        .quad   .Lubsan_type0
        .byte   2   
        .byte   0   
        .zero   6   
        .align 2
        .type   .Lubsan_type0, @object
        .size   .Lubsan_type0, 10
.Lubsan_type0:
        .value  -1  
        .value  0
        .string "'int'"
        .ident  "GCC: (GNU) 14.0.0 20231012 (experimental)"
        .section        .note.GNU-stack,"",@progbits

With -O2 the compiler behaves correctly and produces this:

        .file   "align-5.c"
        .text
        .p2align 4
        .globl  foo 
        .type   foo, @function
foo:
.LFB0:
        .cfi_startproc
        movl    (%rdi), %eax
        ret
        .cfi_endproc
.LFE0:
        .size   foo, .-foo
        .ident  "GCC: (GNU) 14.0.0 20231012 (experimental)"
        .section        .note.GNU-stack,"",@progbits

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug middle-end/111875] With -Og ubsan check inserted even though __builtin_assume_aligned guarantees no UB
  2023-10-19  8:54 [Bug middle-end/111875] New: With -Og ubsan check inserted even though __builtin_assume_aligned guarantees no UB fkastl at suse dot cz
@ 2023-10-19  9:25 ` fkastl at suse dot cz
  2023-10-19  9:41 ` jakub at gcc dot gnu.org
  2023-10-19 13:52 ` rguenth at gcc dot gnu.org
  2 siblings, 0 replies; 4+ messages in thread
From: fkastl at suse dot cz @ 2023-10-19  9:25 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111875

--- Comment #1 from Filip Kastl <fkastl at suse dot cz> ---
I found out that this is caused by the copy_prop pass. With -Og, an instance of
copy_prop runs after the fold_builtins pass but before the sanopt pass. The
fold_builtins pass changes the statement p_2 = __builtin_assume_aligned(p_1, 4)
to p_2 = p_1; and changes the alignment of p_2 to 32 bits. However the
alignment of p_1 remains 8 bits so when copy_prop propagates all occurences of
p_2 to instead be occurences of p_1, the information about alignment is lost.
When the sanopt pass runs, it decides that casting p to (int *) possibly
creates UB.

I see a few possible solutions:
- Stop copy prop from propagating through assignments where the alignments
differ
- Modify copy prop to use the alignment information of the lhs ssa name when
propagating through similar assignment statements
- Modify fold_builtins to copy propagate in similar cases
- Modify fold_builtins to also set alignment of the rhs ssa name when removing
__builtin_assume_aligned in similar cases

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug middle-end/111875] With -Og ubsan check inserted even though __builtin_assume_aligned guarantees no UB
  2023-10-19  8:54 [Bug middle-end/111875] New: With -Og ubsan check inserted even though __builtin_assume_aligned guarantees no UB fkastl at suse dot cz
  2023-10-19  9:25 ` [Bug middle-end/111875] " fkastl at suse dot cz
@ 2023-10-19  9:41 ` jakub at gcc dot gnu.org
  2023-10-19 13:52 ` rguenth at gcc dot gnu.org
  2 siblings, 0 replies; 4+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-10-19  9:41 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111875

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The whole point of the sanitizers is don't trust users as much as possible,
verify there is no UB at runtime.
So, sanitizers shouldn't if possible use VRP etc. in order to optimize checks
away.
We don't achieve that always, best at -O0 of course, but that is the intent.
Furthermore, I think at -Og VRP isn't enabled at all.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Bug middle-end/111875] With -Og ubsan check inserted even though __builtin_assume_aligned guarantees no UB
  2023-10-19  8:54 [Bug middle-end/111875] New: With -Og ubsan check inserted even though __builtin_assume_aligned guarantees no UB fkastl at suse dot cz
  2023-10-19  9:25 ` [Bug middle-end/111875] " fkastl at suse dot cz
  2023-10-19  9:41 ` jakub at gcc dot gnu.org
@ 2023-10-19 13:52 ` rguenth at gcc dot gnu.org
  2 siblings, 0 replies; 4+ messages in thread
From: rguenth at gcc dot gnu.org @ 2023-10-19 13:52 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111875

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
CCP propagates the alignment here.(In reply to Filip Kastl from comment #1)
> I found out that this is caused by the copy_prop pass. With -Og, an instance
> of copy_prop runs after the fold_builtins pass but before the sanopt pass.
> The fold_builtins pass changes the statement p_2 =
> __builtin_assume_aligned(p_1, 4) to p_2 = p_1; and changes the alignment of
> p_2 to 32 bits. However the alignment of p_1 remains 8 bits so when
> copy_prop propagates all occurences of p_2 to instead be occurences of p_1,
> the information about alignment is lost. When the sanopt pass runs, it
> decides that casting p to (int *) possibly creates UB.
> 
> I see a few possible solutions:
> - Stop copy prop from propagating through assignments where the alignments
> differ
> - Modify copy prop to use the alignment information of the lhs ssa name when
> propagating through similar assignment statements
> - Modify fold_builtins to copy propagate in similar cases
> - Modify fold_builtins to also set alignment of the rhs ssa name when
> removing __builtin_assume_aligned in similar cases

I think in general none of those work.  IIRC the copyprop pass was put there
specifically as a "cheap" way to propagate constants exposed by
pass_fold_builtins.  git blame might tell - there was the alternative to
perform this propagation in fold_builtins but it's difficult to be
"complete" there.  The alternative would be to turn that into a proper
simple constant propagation pass.

Not sure if all worth for -Og just because of sanopt though.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-10-19 13:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-19  8:54 [Bug middle-end/111875] New: With -Og ubsan check inserted even though __builtin_assume_aligned guarantees no UB fkastl at suse dot cz
2023-10-19  9:25 ` [Bug middle-end/111875] " fkastl at suse dot cz
2023-10-19  9:41 ` jakub at gcc dot gnu.org
2023-10-19 13:52 ` rguenth at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).