Re: MIPS ASAN status? (and "volunteering")

[Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>]

H.J.: please see last.

> From: Jean Lee <xiaoyur347@gmail.com>
> Date: Sat, 10 Mar 2018 20:22:45 +0800

> > See above regarding looking at patches, but I guess you mean
> > that the patch is trivial, so then I presume it was more or less
> > the same as this, which is basically a copy-paste from looking
> > at rs6000 and xtensa.  I checked a typical /proc/self/maps and
> > guess that 1<<29 probably fits for MIPS o32 too:
> 
> No, Not more or less the same as a copy-paste from rs6000.
> mips_asan_shadow_offset should use 0x0aaa0000 as defined in
> libsanitizer/sanitizer_common/asan_mapping.h
> static const u64 kMIPS32_ShadowOffset32 = 0x0aaa0000;

Thanks!
I really should have inspected the library to verify a match for
my guess for TARGET_ASAN_SHADOW_OFFSET!  That's an odd number though.

> Maybe it is the main reason of your crash.

Yes, it does!  Off to testing now.

> >-                                     FIRST_32_SECOND_64(160, 216);
> >+                                     FIRST_32_SECOND_64(144, 216);
> >If you (or anyone else) has a clue on the kernel-stat size
> >160 vs. 144 issue, or can point out "usual suspects" when always
> >getting SEGV with -fsanitize=address, I'm very interested.
> struct_kernel_stat_sz was once 144 for MIPS but it changes to 160 after
> this commit
> https://llvm.org/viewvc/llvm-project?view=revision&revision=301307

Right, that's what I meant by "the one-line commit of upstream
compiler-rt r301307 set this to 160 for some reason".

> or
> https://github.com/llvm-mirror/compiler-rt/commit/e80e955171a57aa3be9a9d03404a7e9652b8a78d
> Maybe you should ask llvm for help.

Actually, I've got that part figured out.  The compiler-rt
commit of r301307 has the commit-log "[Compiler-rt][MIPS] Fix
assert introduce with commit rl301171."  Now, the rl301171 is
not a typo:ed plain svn revision number, but LLVM-lingo for a
svn commit to LLVM proper.  This commit changed LLVM to compile
with -D_FILE_OFFSET_BITS=64 (thanks to my colleague Rabin
Vincent for this observation).

This should ring a bell: _FILE_OFFSET_BITS affects *user* struct
stat (on 32-bit platforms) but definitely not *kernel* struct
stat.  Indeed, when looking, the bug is apparently that
libsanitizer/sanitizer_common/sanitizer_platform_limits_linux.cc
has:

#if defined(__x86_64__) ||  defined(__mips__)
#include <sys/stat.h>
#else
#define ino_t __kernel_ino_t
#define mode_t __kernel_mode_t
#define nlink_t __kernel_nlink_t
#define uid_t __kernel_uid_t
#define gid_t __kernel_gid_t
#define off_t __kernel_off_t
#define time_t __kernel_time_t
// This header seems to contain the definitions of _kernel_ stat* structs.
#include <asm/stat.h>
#undef ino_t
...
<compiletime-asserting certain values for sizeof(struct stat)>

...and that this wasn't adjusted when -D_FILE_OFFSET_BITS=64 was
added.  For x86_64 this is unimportant, as "struct stat" does
not change size if sizeof(long) == 8 (or rather, struct stat64
doesn't exist).  IMHO, that "#include <sys/stat.h>" should not
be there, for any platform.  I guess it's supposed to be a
workaround for <asm/stat.h> not being compilable at one time for
x86_64 and then the "|| defined(__mips__)" was added later
without deeper analysis, but something else really should be
done and <asm/stat.h> be used everywhere without ifdeffery.
I'll see if I get time to address this or just pile on.
Thankfully sanitizer_platform_limits_linux.cc doesn't do much
besides those sizeof-asserts.

brgds, H-P