This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc 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: [patch] fix uninitialized variable in dynamic linker


Roland McGrath <roland@hack.frob.com> writes:
> If it is indeed right to zero the entire structure, then the cleanest
> way to do that is just to change:
> 
>   struct dl_start_final_info info;
> 
> to:
> 
>   struct dl_start_final_info info = {};
> 
> Then the compiler is responsible for figuring out the memset equivalent.
> 
> Heretofore it never seemed necessary, suggesting that l_info was the
> only part of struct link_map that is not explicitly initialized and that
> is actually consulted during bootstrap relocation.  If that was indeed
> true before, and recent MIPS additions to its l_mach (struct
> link_map_machine) were the only thing that caused any other part of
> bootstrap_map to need initialization, then there is a case to be made
> for initializing only those recently-added fields.  The general logic
> trading off blind zeroing as "future-proofing" vs the performance cost
> has a different calculus here (and often in libc in general), because we
> are talking about adding some static cost to every program's startup
> across the entire system.

Granted this will cost a little more but does at least bring the dynamic
loader link_map in line with a dynamically allocated one created by
_dl_new_object which calloc's the memory. If opinion is swayed to having
an init function or similar then I'm happy to add that to handle the
recent MIPS additions. Perhaps an alternative though would be to
unconditionally initialise the l_mach field which would make it a bit
simpler for ports to understand how that structure can be used. I found it
difficult to figure out if the structure would be zero initialised and
only found the _dl_new_object way of creating it.

Thanks,
Matthew

> Since most machines define PI_STATIC_AND_HIDDEN and so do not use this
> code path (bootstrap_map on the stack), I think we are less concerned
> than we otherwise would be about the blind zeroing doing excess dead
> stores.
> AFAICT the machines that fail to define PI_STATIC_AND_HIDDEN are
> aarch64, arm, hppa, m68k, mips*, powerpc*, tile*.  For aarch64 I suspect
> it is just an oversight.  For arm I suspect it could be enabled
> unconditionally now that we require more recent compilers; certainly it
> could be enabled by a configure test, though I don't know how to write
> one (since I don't know the circumstances in which it might not work).
> For the others I don't personally care about regressions in
> microoptimization.
> 
> 
> Thanks,
> Roland


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