Hi Hans-Peter,
You're right, this is not ok for the O32 ABI. Your change however, broke the 
functionality
for the N32 ABI. AFAIK, the changes like this should go through LLVM first 
(yours didn't),
so I will send out a review, covering both 32 ABIs, meaning that both O32 and 
N32 ABIs 
will be working properly. As for this change, I'm not sure what should be done? 
Should this be committed now, while the LLVM change is cherry-picked once it's 
committed.
Best regards,
Dimitrije Milosevic


From: Hans-Peter Nilsson <h...@bitrange.com>
Sent: Saturday, July 9, 2022 4:44 PM
To: Xi Ruoyao <xry...@xry111.site>
Cc: Dimitrije Milosevic <dimitrije.milose...@syrmia.com>; Djordje Todorovic 
<djordje.todoro...@syrmia.com>; gcc-patches@gcc.gnu.org 
<gcc-patches@gcc.gnu.org>
Subject: Re: Mips: Fix kernel_stat structure size 
 
On Sat, 9 Jul 2022, Xi Ruoyao wrote:

> On Fri, 2022-07-08 at 21:42 -0400, Hans-Peter Nilsson wrote:
> > On Fri, 1 Jul 2022, Dimitrije Milosevic wrote:
> >
> > > Fix kernel_stat structure size for non-Android 32-bit Mips.
> > > LLVM currently has this value for the kernel_stat structure size,
> > > as per compiler-rt/lib/sanitizer-
> > > common/sanitizer_platform_limits_posix.h.
> > > This also resolves one of the build issues for non-Android 32-bit
> > > Mips.
> >
> > I insist that PR105614 comment #7 is the way to go, i.e. fix
> > the merge error, avoiding the faulty include that it
> > reintroduced.? Was this tested on O32?
>
> I'm pretty sure it is *not* the way to go.
>
> Sanitizer does not really intercept system call.  It intercepts libc
> stat() or lstat() etc. calls.  So you need to keep struct_kernel_stat_sz
> same as the size of struct stat in libc, i. e. "the size of buffer which
> *libc* stat()-like functions writing into".
>
> The "kernel_" in the name is just misleading.

You make a sound argument, and I just refer to my old commit
9f943b2446f2d0a.  Either way, the proof is in the pussing: was
this tested for O32 or not?

> And, if you still think it should be the way to go, let's submit the
> change to LLVM and get it reviewed properly.

Sorry, I've no longer interest in mips besides I'd like to see
un-broken what I helped getting in a working state (support ASAN
in gcc for mips O32).

brgds, H-P

Reply via email to