On 22 Jan 2018 09:23, Yunlian Jiang wrote:
> On Fri, Jan 19, 2018 at 3:32 PM, Mike Frysinger wrote:
> > On 18 Jan 2018 13:48, Yunlian Jiang wrote:
> > >    I tried to build busybox with clang and use it to create recovery
> > image
> > > for ChromeOS.
> > > It fails to recover an arm based ChromeBook.
> >
> > is it that busybox is crashing ?  is clang/lld placing this pointer in
> > const
> > memory (even though we have section(".data")) ?  or is it generating an
> > abort
> > when it reaches what it thinks is an undefined situation (like trying to
> > write
> > to a const memory location in INIT_G_var()) ?
> >
> The busybox is crashing. On arm boards, the command line
> 'busybox ash' gave me a segmentation fault.
> 
> In the ash_ptr_hack.c, it sets the attribute  __attribute__ (section
> (".data")) when
> 'GCC_COMBINE' is defined.  But clang does not have this MACRO defined.

i don't think GCC_COMBINE is relevant, so just forget about it.  ash_ptr_hack.c
does:
  struct globals_misc *ash_ptr_to_globals_misc;
that means that pointer should end up the .data/writable memory by default.

the fact that another compilation unit declares it as const isn't terribly
relevant ... the actual storage should still be writable.  you should check
that is happening (or if it's ending up somewhere else).  we've seen llvm
incorrectly propagate attributes to storage before based on decls seen in
other modules.

> > >    I digged a little bit.
> > >    Below patch makes it work.
> >
> > your patch only changed two places ... presumably if this global ptr
> > trickery
> > isn't working here, it doesn't work in any of them.
> >
> My patch changed the the declarations in the ash.c, did I miss something?

if you look in ash_ptr_hack.c, we have three pointers doing this kind of
trickery, but you only changed two.  if you grep the tree for GCC_COMBINE
or const.*attribute.*section.*data, you see we use the same trick in other
places in busybox.  so if it's not working in ash, i don't expect it to
work for any of these.

> > > My question is.
> > >    the  ash_ptr_to_globals_misc, ash_ptr_to_globals_memstack
> > > and ash_ptr_to_globals_var
> > > are defined in ash_ptr_hack.c as normal pointers. But in ash.c, they are
> > > declared as const
> > > pointers. What is the benefit of doing that?
> >
> > the pointer itself is the thing that is const, not the memory it points to.
> > this lets the compiler optimize the loads by generating relocations via the
> > pointer ... there's the fixup at the initial load time, but after that,
> > it's
> > just offsets to a constant memory location.  but removing the const
> > markings,
> > the compiler has to reload the pointer everytime.
>
> Can this trick be included only in configurations for smaller code size?

before we try doing something like that, we really need to understand why it's
failing in the first place.  we don't want to just shove things under the carpet
and hope no one notices the bumps.
-mike

Attachment: signature.asc
Description: Digital signature

_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox

Reply via email to