This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: libsanitizer merge from upstream r208536


On Thu, May 22, 2014 at 06:34:22PM +0400, Konstantin Serebryany wrote:
> > Notes:
> > 1) the cases where we e.g. for various stringops perform first and
> >    last address check and use separate __asan_report_*1 for reporting
> >    that should probably be converted to use this __asan_report_*_n too
> 
> Note that in clang we completely removed the code that instruments
> srtingops (we had memset/memcpy/memmove).
> Instead we replace memset with __asan_memset (ditto for
> memcpy/memmove) so that it does not get inlined later.
> This simplifies the code and allows to properly analyze all memset/etc
> calls, not just check the first and the last bytes.

Food for thought, yes.

> > 3) there is still a failure for -m32:
> > FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_UAF_long_double Ident(p)[12] = 0 output pattern test
> > Output should match: WRITE of size 1[06]
> > FAIL: g++.dg/asan/asan_test.C  -O2  AddressSanitizer_UAF_long_double Ident(p)[0] = Ident(p)[12] output pattern test
> > Output should match: READ of size 1[06]
> >    That sounds like something to fix in upstream, it should allow also size
> >    12 which is the size of long double on ia32 (16 bytes on x86_64),
> >    thus 1[026].  Kostya, can you please change it, I'll then apply it
> >    to the testsuite patch too?
> Like this?
> 
> --- lib/asan/tests/asan_test.cc (revision 209430)
> +++ lib/asan/tests/asan_test.cc (working copy)
> @@ -183,8 +183,8 @@
>  TEST(AddressSanitizer, UAF_long_double) {
>    if (sizeof(long double) == sizeof(double)) return;
>    long double *p = Ident(new long double[10]);
> -  EXPECT_DEATH(Ident(p)[12] = 0, "WRITE of size 1[06]");
> -  EXPECT_DEATH(Ident(p)[0] = Ident(p)[12], "READ of size 1[06]");
> +  EXPECT_DEATH(Ident(p)[12] = 0, "WRITE of size 1[026]");
> +  EXPECT_DEATH(Ident(p)[0] = Ident(p)[12], "READ of size 1[026]");
>    delete [] Ident(p);
>  }
> 

Yep, exactly.

> > As mentioned earlier, ubsan has similar
> >    problem where it doesn't recognize float bitsize 96 (but unlike this
> >    case where clang+llvm pass in 10 bytes, which is what is actually
> >    accessed by hw if using i387 stack, but not if using other means of
> >    copying it, in ubsan case clang also passes in bitsize 96 that ubsan
> >    doesn't handle).
> 
> Yea, this long double business is rather confusing to me...

It is actually even more complicated than that, on x86_64 GCC supports
long double (== __float80) which has 128 bits (64-bit) or 96 bits (32-bit),
and is the 80-bit extended double format, plus also __float128, which
is a software implemented IEEE 754 quad long double (also 128 bit).
Now, ubsan handles all of 80 and 128 bit (and should 96 bit) floats
as long double, so it is unclear how to tell libubsan about __float128
type (because bitsize 128 is already used for something else),
plus in order to actually print __float128 one would need to link in
libquadmath, which is probably not desirable.  So, ideally there should
be some way to tell the library we have IEEE 754 quad __float128 and
for now the library should just print that the number is not printable.
Ditto for _Decimal{32,64,128}, which should probably just be a different
kind from float, and again would require additional library to support
printing into strings, so perhaps it could be for now just printed
as something unprintable.

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]