[PATCH] D65819: [Driver][Bundler] Improve bundling of object files.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D65819#1627693 <https://reviews.llvm.org/D65819#1627693>, @Hahnfeld wrote:

> In D65819#1627638 <https://reviews.llvm.org/D65819#1627638>, @ABataev wrote:
>
> > In D65819#1627631 <https://reviews.llvm.org/D65819#1627631>, @Hahnfeld wrote:
> >
> > > In D65819#1627620 <https://reviews.llvm.org/D65819#1627620>, @ABataev wrote:
> > >
> > > > In D65819#1627600 <https://reviews.llvm.org/D65819#1627600>, @Hahnfeld wrote:
> > > >
> > > > > In D65819#1627036 <https://reviews.llvm.org/D65819#1627036>, @ABataev wrote:
> > > > >
> > > > > > In D65819#1627032 <https://reviews.llvm.org/D65819#1627032>, @Hahnfeld wrote:
> > > > > >
> > > > > > > In D65819#1617736 <https://reviews.llvm.org/D65819#1617736>, @Hahnfeld wrote:
> > > > > > >
> > > > > > > > Will this patch change the ability to consume a bundled object file without calling the unbundler? Using known ELF tools on the produced object files was an important design decision and IIRC was somewhat important for using build systems that are unaware of the bundled format.
> > > > > > >
> > > > > > >
> > > > > > > Ping.
> > > > > >
> > > > > >
> > > > > > Missed this. We still produce correct object files, so all the tools will work with this.
> > > > >
> > > > >
> > > > > I agree on a technical level that it's still a "correct" object, but not what I was looking for: The host object file will only be in the bundled section, so you cannot examine it without unbundling.
> > > > >
> > > > > For example, with a small test file and `clang -fopenmp -fopenmp-targets=x86_64 -c test.c` I saw the following:
> > > > >
> > > > >   $ nm test.o                                       
> > > > >   0000000000000000 t .omp_offloading.requires_reg
> > > > >   0000000000000000 T test
> > > > >                    U __tgt_register_requires
> > > > >
> > > > >
> > > > > After applying this patch, the output is empty which might be a problem in certain cases.
> > > >
> > > >
> > > > Unfortunately, this is the only possible solution I see. There are already 2 reports that bundled objects does not work correctly after unbundling.
> > >
> > >
> > > Can you please again share what exactly is the problem, with a small example? I saw discussions on openmp-dev, but that project was huge, and above you were quoting a man page and hinted to global constructors.
> >
> >
> > I don't have a small reproducer, unfortunately, only the big one. 
> >  Here is the message from the user:
> >
> >   Hi,
> >   I am revisiting this possible compiler bug in Clang 8.0.0.
> >  
> >   In the source code I am developing, there's a global static variable, nest::sli_neuron::recordablesMap_ put in the BSS section and it is expected to be fully initialized by the time nest::sli_neuron::sli_neuron() gets called, however in a gdb session:
> >  
> >   (gdb) p nest::sli_neuron::recordablesMap_
> >   Python Exception <type 'exceptions.AttributeError'> 'gdb.Type' object has no attribute 'name':
> >   $1 = {<std::map<Name, double (nest::sli_neuron::*)() const, std::less<Name>, std::allocator<std::pair<Name const, double (nest::sli_neuron::*)() const> > >> = std::map with 0 elements, _vptr$RecordablesMap = 0x0}
> >  
> >   this doesn't happen when -fopenmp-targets is _not_ used, is it not trivial to come up a reproducer, thus I am sending a work-in-progress report hoping someone will shed some light on this.
> >  
> >   Thanks,
> >   Itaru.
> >
> >
> > Another one:
> >
> >   Hi,
> >   I am seeing a link error shown below:
> >  
> >   `.text.startup' referenced in section `.init_array.0' of /tmp/event_delivery_manager-02f392.o: defined in discarded section `.text.startup[_ZN4nest18DataSecondaryEventIdNS_16GapJunctionEventEE18supported_syn_ids_E]' of /tmp/event_delivery_manager-02f392.o
> >   clang-10: error: linker command failed with exit code 1 (use -v to see invocation)
> >  
> >   I am not sure how to tackle this as the part is referenced isn't what I am
> >   working on. I am using the latest trunk Clang 10 at this moment.
> >
> >
> > Steps to reproduce:
> >
> >   Steps to produce:
> >   $ git clone https://https://github.com/nest/nest-simulator/
> >   $ mkdir /tmp/build/nest-clang-offload/
> >   $ cd /tmp/build/nest-clang-offload/
> >   $ cmake -DCMAKE_TOOLCHAIN_FILE=Platform/JURON_Clang -DCMAKE_INSTALL_PREFIX=/path/to/opt/nest-clang -Dwith-gsl=ON -Dwith-openmp=ON -Dwith-mpi=OFF -Dwith-python=OFF -Dwith-offload=ON /path/to/nest-simulator/
> >
>
>
> I've seen these reports. What has eventually led to this patch, ie where do constructors come into play?


When disabled partial linking and linked the original object file instead of the "unbundled" one manually, the global variable is initialized properly. It is a global variable which must call the constructor upon program start. With partial linking, this constructor is not called and the variable remains zeroinitialized.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65819/new/

https://reviews.llvm.org/D65819