Bug 102656 - [11 Regression] ICE on coroutines on -fsanitize=address -O1 since r11-1613-g788b962aa00959e8
Summary: [11 Regression] ICE on coroutines on -fsanitize=address -O1 since r11-1613-g7...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: sanitizer (show other bugs)
Version: 11.2.1
: P2 normal
Target Milestone: 11.3
Assignee: Jakub Jelinek
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2021-10-08 21:03 UTC by Dmitry Prokoptsev
Modified: 2022-05-10 08:24 UTC (History)
7 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2021-10-11 00:00:00


Attachments
gcc12-pr102656.patch (1.06 KB, patch)
2022-02-17 16:17 UTC, Jakub Jelinek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dmitry Prokoptsev 2021-10-08 21:03:05 UTC
Here's the minimal example I've been able to come up with:

// ICEs on -fcoroutines -O1 -fsanitize=address, on any gcc version between 10.2 and trunk (does compile though on 10.1), checked on godbolt.

#include <coroutine>
/* 2*/ class promise;
/* 3*/
/* 4*/ struct future {
/* 5*/     using promise_type = promise;
/* 6*/     future() = default;
/* 7*/     int x = 0;  // commenting out this line resolves the ICE
/* 8*/ };
/* 9*/ 
/*10*/ class promise {
/*11*/  public:
/*12*/    future get_return_object() noexcept { return {}; }
/*13*/    auto initial_suspend() noexcept { return std::suspend_never{}; }
/*14*/    auto final_suspend() noexcept { return std::suspend_never{}; }
/*15*/
/*16*/    void return_void() noexcept {}
/*17*/    void unhandled_exception() {}
/*18*/};
/*19*/
/*20*/ future func() { co_return; }

gcc-10.2 [x86-64] yields
<source>:20:28: internal compiler error: in expand_expr_addr_expr_1, at expr.c:8075
   20 | future func() { co_return; }
Comment 1 Dmitry Prokoptsev 2021-10-08 21:30:00 UTC
Follow-up: adding a non-trivial copy constructor --
    future(const future& f): x(f.x) {}
-- also resolves the ICE.
Comment 2 Martin Liška 2021-10-11 10:39:48 UTC
Confirmed, started with r11-1613-g788b962aa00959e8.
Comment 3 Richard Biener 2021-11-05 13:25:21 UTC
Not sure if really a regression - the rev. is from 11 development time.
Comment 4 Jakub Jelinek 2022-02-17 16:17:30 UTC
Created attachment 52461 [details]
gcc12-pr102656.patch

Untested fix.
Comment 5 GCC Commits 2022-02-19 08:05:08 UTC
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:9e3bbb4a8024121eb0fa675cb1f074218c1345a6

commit r12-7300-g9e3bbb4a8024121eb0fa675cb1f074218c1345a6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Feb 19 09:03:57 2022 +0100

    asan: Mark instrumented vars addressable [PR102656]
    
    We ICE on the following testcase, because the asan1 pass decides to
    instrument
      <retval>.x = 0;
    and does that by
      _13 = &<retval>.x;
      .ASAN_CHECK (7, _13, 4, 4);
      <retval>.x = 0;
    and later sanopt pass turns that into:
      _39 = (unsigned long) &<retval>.x;
      _40 = _39 >> 3;
      _41 = _40 + 2147450880;
      _42 = (signed char *) _41;
      _43 = *_42;
      _44 = _43 != 0;
      _45 = _39 & 7;
      _46 = (signed char) _45;
      _47 = _46 + 3;
      _48 = _47 >= _43;
      _49 = _44 & _48;
      if (_49 != 0)
        goto <bb 10>; [0.05%]
      else
        goto <bb 9>; [99.95%]
    
      <bb 10> [local count: 536864]:
      __builtin___asan_report_store4 (_39);
    
      <bb 9> [local count: 1073741824]:
      <retval>.x = 0;
    The problem is during expansion, <retval> isn't marked TREE_ADDRESSABLE,
    even when we take its address in (unsigned long) &<retval>.x.
    
    Now, instrument_derefs has code to avoid the instrumentation altogether
    if we can prove the access is within bounds of an automatic variable in the
    current function and the var isn't TREE_ADDRESSABLE (or we don't instrument
    use after scope), but we do it solely for VAR_DECLs.
    
    I think we should treat RESULT_DECLs exactly like that too, which is what
    the following patch does.  I must say I'm unsure about PARM_DECLs, those can
    have different cases, either they are fully or partially passed in
    registers, then if we take parameter's address, they are in a local copy
    inside of a function and so work like those automatic vars.  But if they
    are fully passed in memory, we typically just take address of the slot
    and in that case they live in the caller's frame.  It is true we don't
    (can't) put any asan padding in between the arguments, so all asan could
    detect in that case is if caller passes fewer on stack arguments or smaller
    arguments than callee accepts.  Anyway, as I'm unsure, I haven't added
    PARM_DECLs to that case.
    
    And another thing is, when we actually build_fold_addr_expr, we need to
    mark_addressable the inner if it isn't addressable already.
    
    2022-02-19  Jakub Jelinek  <jakub@redhat.com>
    
            PR sanitizer/102656
            * asan.cc (instrument_derefs): If inner is a RESULT_DECL and access is
            known to be within bounds, treat it like automatic variables.
            If instrumenting access and inner is {VAR,PARM,RESULT}_DECL from
            current function and !TREE_STATIC which is not TREE_ADDRESSABLE, mark
            it addressable.
    
            * g++.dg/asan/pr102656.C: New test.
Comment 6 Jakub Jelinek 2022-02-19 08:09:57 UTC
Fixed on the trunk so far.
Comment 7 GCC Commits 2022-03-29 05:53:07 UTC
The releases/gcc-11 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:92374fd237cdbdf4d2c08ffb216d2ebcc799dc4b

commit r11-9716-g92374fd237cdbdf4d2c08ffb216d2ebcc799dc4b
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Feb 19 09:03:57 2022 +0100

    asan: Mark instrumented vars addressable [PR102656]
    
    We ICE on the following testcase, because the asan1 pass decides to
    instrument
      <retval>.x = 0;
    and does that by
      _13 = &<retval>.x;
      .ASAN_CHECK (7, _13, 4, 4);
      <retval>.x = 0;
    and later sanopt pass turns that into:
      _39 = (unsigned long) &<retval>.x;
      _40 = _39 >> 3;
      _41 = _40 + 2147450880;
      _42 = (signed char *) _41;
      _43 = *_42;
      _44 = _43 != 0;
      _45 = _39 & 7;
      _46 = (signed char) _45;
      _47 = _46 + 3;
      _48 = _47 >= _43;
      _49 = _44 & _48;
      if (_49 != 0)
        goto <bb 10>; [0.05%]
      else
        goto <bb 9>; [99.95%]
    
      <bb 10> [local count: 536864]:
      __builtin___asan_report_store4 (_39);
    
      <bb 9> [local count: 1073741824]:
      <retval>.x = 0;
    The problem is during expansion, <retval> isn't marked TREE_ADDRESSABLE,
    even when we take its address in (unsigned long) &<retval>.x.
    
    Now, instrument_derefs has code to avoid the instrumentation altogether
    if we can prove the access is within bounds of an automatic variable in the
    current function and the var isn't TREE_ADDRESSABLE (or we don't instrument
    use after scope), but we do it solely for VAR_DECLs.
    
    I think we should treat RESULT_DECLs exactly like that too, which is what
    the following patch does.  I must say I'm unsure about PARM_DECLs, those can
    have different cases, either they are fully or partially passed in
    registers, then if we take parameter's address, they are in a local copy
    inside of a function and so work like those automatic vars.  But if they
    are fully passed in memory, we typically just take address of the slot
    and in that case they live in the caller's frame.  It is true we don't
    (can't) put any asan padding in between the arguments, so all asan could
    detect in that case is if caller passes fewer on stack arguments or smaller
    arguments than callee accepts.  Anyway, as I'm unsure, I haven't added
    PARM_DECLs to that case.
    
    And another thing is, when we actually build_fold_addr_expr, we need to
    mark_addressable the inner if it isn't addressable already.
    
    2022-02-19  Jakub Jelinek  <jakub@redhat.com>
    
            PR sanitizer/102656
            * asan.c (instrument_derefs): If inner is a RESULT_DECL and access is
            known to be within bounds, treat it like automatic variables.
            If instrumenting access and inner is {VAR,PARM,RESULT}_DECL from
            current function and !TREE_STATIC which is not TREE_ADDRESSABLE, mark
            it addressable.
    
            * g++.dg/asan/pr102656.C: New test.
    
    (cherry picked from commit 9e3bbb4a8024121eb0fa675cb1f074218c1345a6)
Comment 8 Jakub Jelinek 2022-03-30 08:14:23 UTC
Fixed for 11.3 too.
Comment 9 GCC Commits 2022-05-10 08:24:29 UTC
The releases/gcc-10 branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:f2549d3bc752bf832ca6de9830ff50b3f23143ab

commit r10-10686-gf2549d3bc752bf832ca6de9830ff50b3f23143ab
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Sat Feb 19 09:03:57 2022 +0100

    asan: Mark instrumented vars addressable [PR102656]
    
    We ICE on the following testcase, because the asan1 pass decides to
    instrument
      <retval>.x = 0;
    and does that by
      _13 = &<retval>.x;
      .ASAN_CHECK (7, _13, 4, 4);
      <retval>.x = 0;
    and later sanopt pass turns that into:
      _39 = (unsigned long) &<retval>.x;
      _40 = _39 >> 3;
      _41 = _40 + 2147450880;
      _42 = (signed char *) _41;
      _43 = *_42;
      _44 = _43 != 0;
      _45 = _39 & 7;
      _46 = (signed char) _45;
      _47 = _46 + 3;
      _48 = _47 >= _43;
      _49 = _44 & _48;
      if (_49 != 0)
        goto <bb 10>; [0.05%]
      else
        goto <bb 9>; [99.95%]
    
      <bb 10> [local count: 536864]:
      __builtin___asan_report_store4 (_39);
    
      <bb 9> [local count: 1073741824]:
      <retval>.x = 0;
    The problem is during expansion, <retval> isn't marked TREE_ADDRESSABLE,
    even when we take its address in (unsigned long) &<retval>.x.
    
    Now, instrument_derefs has code to avoid the instrumentation altogether
    if we can prove the access is within bounds of an automatic variable in the
    current function and the var isn't TREE_ADDRESSABLE (or we don't instrument
    use after scope), but we do it solely for VAR_DECLs.
    
    I think we should treat RESULT_DECLs exactly like that too, which is what
    the following patch does.  I must say I'm unsure about PARM_DECLs, those can
    have different cases, either they are fully or partially passed in
    registers, then if we take parameter's address, they are in a local copy
    inside of a function and so work like those automatic vars.  But if they
    are fully passed in memory, we typically just take address of the slot
    and in that case they live in the caller's frame.  It is true we don't
    (can't) put any asan padding in between the arguments, so all asan could
    detect in that case is if caller passes fewer on stack arguments or smaller
    arguments than callee accepts.  Anyway, as I'm unsure, I haven't added
    PARM_DECLs to that case.
    
    And another thing is, when we actually build_fold_addr_expr, we need to
    mark_addressable the inner if it isn't addressable already.
    
    2022-02-19  Jakub Jelinek  <jakub@redhat.com>
    
            PR sanitizer/102656
            * asan.c (instrument_derefs): If inner is a RESULT_DECL and access is
            known to be within bounds, treat it like automatic variables.
            If instrumenting access and inner is {VAR,PARM,RESULT}_DECL from
            current function and !TREE_STATIC which is not TREE_ADDRESSABLE, mark
            it addressable.
    
    (cherry picked from commit 9e3bbb4a8024121eb0fa675cb1f074218c1345a6)