Patchwork [asan] Patch - fix an ICE in asan.c

login
register
mail settings
Submitter Tobias Burnus
Date Nov. 9, 2012, 8:36 p.m.
Message ID <509D6965.5040405@net-b.de>
Download mbox | patch
Permalink /patch/198149/
State New
Headers show

Comments

Tobias Burnus - Nov. 9, 2012, 8:36 p.m.
The attached test case ICEs (segfault) both on the asan branch and on 
the trunk with Dodji's patches:

fail31.ii: In static member function 'static std::size_t 
std::char_traits<char>::length(const char_type*)':
fail31.ii:13:19: internal compiler error: Segmentation fault
      static size_t length (const char_type * __s)
                    ^
0xae02ef crash_signal
         /projects/tob/gcc-git/gcc/gcc/toplev.c:334
0xaf031d gsi_next
         /projects/tob/gcc-git/gcc/gcc/gimple.h:5072
0xaf031d transform_statements
         /projects/tob/gcc-git/gcc/gcc/asan.c:1357
0xaf031d asan_instrument
         /projects/tob/gcc-git/gcc/gcc/asan.c:1556



The problem is in asa.c's transform_statements:

   FOR_EACH_BB (bb)
     {
       if (bb->index >= saved_last_basic_block) continue;
       for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
         {
           gimple s = gsi_stmt (i);

           if (gimple_assign_single_p (s))
             instrument_assignment (&i);
           else if (is_gimple_call (s))
             maybe_instrument_call (&i);
     }


Here, "gsi_end_p(i)" is the check "i->ptr == NULL" and gsi_next(&i) is 
"i->ptr = i->ptr->gsbase.next;"

Thus, it looks fine at a glance. However, the problem is that the 
gsi_end_p check is done before the loop body while "gsi_next" is called 
after the loop body. That's fine unless "i" is modified in between, 
which happens in

instrument_strlen_call (gimple_stmt_iterator *iter)
...
   gimple_stmt_iterator gsi = *iter;
...
   *iter = gsi;
}

After the call, iter->ptr == NULL.


Is the patch okay for the ASAN branch?*

Tobias

* I still have to do an all-language bootstrap and regtesting, though 
the latter is probably pointless as there is currently not a single 
-fasan test case.
namespace std
{
  template < typename _Alloc > class allocator;
  template < class _CharT > struct char_traits;
    template < typename _CharT, typename _Traits =
    char_traits < _CharT >, typename _Alloc =
    allocator < _CharT > >class basic_string;
  typedef basic_string < char >string;
  typedef long unsigned int size_t;
    template <> struct char_traits <char >
  {
    typedef char char_type;
    static size_t length (const char_type * __s)
    {
      return __builtin_strlen (__s);
    }
  };
  namespace __gnu_cxx
  {
    template < typename _Tp > class new_allocator
    {
    public:
      typedef size_t size_type;
        template < typename _Tp1 > struct rebind
      {
	typedef new_allocator < _Tp1 > other;
      };
    };
  }
template < typename _Tp > class allocator:public __gnu_cxx::new_allocator <
    _Tp >
  {
  };
  template < typename _CharT, typename _Traits, typename _Alloc >
    class basic_string
  {
    typedef typename _Alloc::template rebind <
      _CharT >::other _CharT_alloc_type;
    typedef _Traits traits_type;
    typedef typename _CharT_alloc_type::size_type size_type;
  public:
    basic_string & operator= (const _CharT * __s)
    {
      return this->assign (__s, traits_type::length (__s));
    }
    basic_string & assign (const _CharT * __s, size_type __n);
  };

  class Regex
  {
    std::string sub (std::string * Error);
  };

  std::string Regex::sub (std::string * Error)
  {
    *Error = "";
  }
}
Tobias Burnus - Nov. 9, 2012, 10 p.m.
Tobias Burnus wrote:
> The attached test case ICEs (segfault) both on the asan branch and on
> the trunk with Dodji's patches:

I found another ICE - this time without a patch.

[That's with the patch, which I posted in this thread. Without, one 
seems to run into the problem I tried to fix with the patch.]

[As ASAN is not yet in the trunk, it is not yet suitable for a PR - but 
on the other hand, I am afraid that I loose it. Thus, I dump it here, 
which is also not the best place (sorry).]


The attached code generates (before ASAN):

StringSwitch<T, R>& StringSwitch ...
...
   <bb 2>:
   _2 = &this_1(D)->Str;
   _3 = StringRef::data (_2);
   memcmp (S_4(D), _3, 7);
   return;
}

And within this basic block, between "_3" and "memcpy", the generated 
ASAN code is added, which leads to an ICE and 10 times the message.


error: control flow in the middle of basic block 7


If one looks at the asan0 dump (after disabling this part of checking), 
one finds:

   _52 = _48 & _51;
   if (_52 != 0)
   _53 = (unsigned long) _22;

The "if" line looks odd as one would expect code of this form:

   if (_62 != 0)
     goto <bb 12>;
   else
     goto <bb 11>;

See attachments.

Tobias
extern "C"
{
  extern int memcmp (const void *__s1, const void *__s2,
		     long unsigned int __n) throw ();
}
struct StringRef
{
  const char *data () const { }
};
template < typename T, typename R = T > class StringSwitch
{
  StringRef Str;
  public:
     explicit StringSwitch (StringRef Str):Str (Str) { }
  template < unsigned N > StringSwitch & Case (const char (&S)[N])
  {
    memcmp (S, Str.data (), N - 1);
  }
  R Default () const { }
};

int getIt (StringRef Name)
{
  return StringSwitch < int > (Name)
    .Case ("unknown")
    .Default ();
}
;; Function const char* StringRef::data() const (_ZNK9StringRef4dataEv, funcdef_no=0, decl_uid=2200, cgraph_uid=0)

const char* StringRef::data() const (const struct StringRef * const this)
{
  <bb 2>:
  GIMPLE_NOP
  return;

}



;; Function int getIt(StringRef) (_Z5getIt9StringRef, funcdef_no=4, decl_uid=2230, cgraph_uid=1)

int getIt(StringRef) (struct StringRef Name)
{
  struct StringSwitch & D.2293;
  struct StringRef D.2292;
  struct StringRef D.2277;
  struct StringSwitch D.2278;
  int D.2291;
  struct StringSwitch & _1;
  int _2;

  <bb 2>:
  StringSwitch<int>::StringSwitch (&D.2278, D.2292);
  _1 = StringSwitch<int>::Case<8u> (&D.2278, "unknown");
  _2 = StringSwitch<int>::Default (_1);
  D.2278 ={v} {CLOBBER};

<L1>:
  return _2;

}



;; Function StringSwitch<T, R>::StringSwitch(StringRef) [with T = int; R = int] (_ZN12StringSwitchIiiEC2E9StringRef, funcdef_no=6, decl_uid=2249, cgraph_uid=3)

StringSwitch<T, R>::StringSwitch(StringRef) [with T = int; R = int] (struct StringSwitch * const this, struct StringRef Str)
{
  <bb 2>:
  return;

}



;; Function StringSwitch<T, R>& StringSwitch<T, R>::Case(const char (&)[N]) [with unsigned int N = 8u; T = int; R = int] (_ZN12StringSwitchIiiE4CaseILj8EEERS0_RAT__Kc, funcdef_no=8, decl_uid=2279, cgraph_uid=5)

StringSwitch<T, R>& StringSwitch<T, R>::Case(const char (&)[N]) [with unsigned int N = 8u; T = int; R = int] (struct StringSwitch * const this, const char[8] & S)
{
  const char * D.2297;
  struct StringRef * D.2296;
  struct StringRef * _2;
  const char * _3;
  unsigned long _5;
  unsigned long _6;
  unsigned long _7;
  signed char * _8;
  signed char _9;
  bool _10;
  unsigned long _11;
  signed char _12;
  bool _13;
  bool _14;
  long unsigned int _15;
  long unsigned int _16;
  const char[8] & _17;
  const char[8] & _18;
  unsigned long _19;
  unsigned long _20;
  unsigned long _21;
  signed char * _22;
  signed char _23;
  bool _24;
  unsigned long _25;
  signed char _26;
  bool _27;
  bool _28;
  unsigned long _29;
  unsigned long _30;
  unsigned long _31;
  signed char * _32;
  signed char _33;
  bool _34;
  unsigned long _35;
  signed char _36;
  bool _37;
  bool _38;
  long unsigned int _39;
  long unsigned int _40;
  const char * _41;
  const char * _42;
  unsigned long _43;
  unsigned long _44;
  unsigned long _45;
  signed char * _46;
  signed char _47;
  bool _48;
  unsigned long _49;
  signed char _50;
  bool _51;
  bool _52;
  unsigned long _53;
  unsigned long _54;
  unsigned long _55;
  signed char * _56;
  signed char _57;
  bool _58;
  unsigned long _59;
  signed char _60;
  bool _61;
  bool _62;

  <bb 2>:
  _2 = &this_1(D)->Str;
  _3 = StringRef::data (_2);
  _5 = (unsigned long) S_4(D);
  _6 = _5 >> 3;
  _7 = _6 + 17592186044416;
  _8 = (signed char *) _7;
  _9 = *_8;
  _10 = _9 != 0;
  _11 = _5 & 7;
  _12 = (signed char) _11;
  _13 = _12 >= _9;
  _14 = _10 & _13;
  if (_14 != 0)
    goto <bb 4>;
  else
    goto <bb 3>;

  <bb 4>:
  __asan_report_load1 (_5);

  <bb 3>:
  _29 = (unsigned long) _3;
  _30 = _29 >> 3;
  _31 = _30 + 17592186044416;
  _32 = (signed char *) _31;
  _33 = *_32;
  _34 = _33 != 0;
  _35 = _29 & 7;
  _36 = (signed char) _35;
  _37 = _36 >= _33;
  _38 = _34 & _37;
  if (_38 != 0)
    goto <bb 8>;
  else
    goto <bb 7>;

  <bb 8>:
  __asan_report_load1 (_29);

  <bb 7>:
  _39 = 7;
  _40 = _39 - 1;
  _41 = _3;
  _42 = _41 + _40;
  _43 = (unsigned long) _42;
  _44 = _43 >> 3;
  _45 = _44 + 17592186044416;
  _46 = (signed char *) _45;
  _47 = *_46;
  _48 = _47 != 0;
  _49 = _43 & 7;
  _50 = (signed char) _49;
  _51 = _50 >= _47;
  _52 = _48 & _51;
  if (_52 != 0)
  _53 = (unsigned long) _22;
  _54 = _53 >> 3;
  _55 = _54 + 17592186044416;
  _56 = (signed char *) _55;
  _57 = *_56;
  _58 = _57 != 0;
  _59 = _53 & 7;
  _60 = (signed char) _59;
  _61 = _60 >= _57;
  _62 = _58 & _61;
  if (_62 != 0)
    goto <bb 12>;
  else
    goto <bb 11>;

  <bb 12>:
  __asan_report_load1 (_53);

  <bb 11>:

  <bb 10>:
  __asan_report_load1 (_43);

  <bb 9>:
  _15 = 7;
  _16 = _15 - 1;
  _17 = S_4(D);
  _18 = _17 + _16;
  _19 = (unsigned long) _18;
  _20 = _19 >> 3;
  _21 = _20 + 17592186044416;
  _22 = (signed char *) _21;
  _23 = *_22;
  _24 = _23 != 0;
  _25 = _19 & 7;
  _26 = (signed char) _25;
  _27 = _26 >= _23;
  _28 = _24 & _27;
  if (_28 != 0)
    goto <bb 6>;
  else
    goto <bb 5>;

  <bb 6>:
  __asan_report_load1 (_19);

  <bb 5>:
  memcmp (S_4(D), _3, 7);
  return;

}
Dodji Seketeli - Nov. 12, 2012, 11:51 a.m.
Tobias Burnus <burnus@net-b.de> writes:

> The attached test case ICEs (segfault) both on the asan branch and on
> the trunk with Dodji's patches:

Thank you for reporting this.

I believe the neqw series of patches I have juste posted address this
issue.

To ease the testing, you can check out an updated git tree with all
these patches cleanly stacked on top of trunk by doing:

    git clone -b asan-merge-assemble git://seketeli.net/~dodji/gcc.git

You can browse the patchset it via the web at: 

    http://seketeli.net/git/~dodji/gcc.git/log/?h=asan-merge-assemble

Cheers.

Patch

--- gcc/asan.c.orig	2012-11-09 21:26:26.000000000 +0100
+++ gcc/asan.c	2012-11-09 21:26:00.000000000 +0100
@@ -1362,6 +1362,8 @@  transform_statements (void)
 	    instrument_assignment (&i);
 	  else if (is_gimple_call (s))
 	    maybe_instrument_call (&i);
+	  if (gsi_end_p (i))
+	    break;
         }
     }
 }