[llvm-commits] COFF patch

[Michael Spencer]

On Mon, Aug 2, 2010 at 5:18 PM, Cameron Esfahani <dirty at apple.com> wrote:
>
> On Jul 31, 2010, at 1:02 AM, Michael Spencer wrote:
>
>> On Thu, Jul 29, 2010 at 7:56 PM, Cameron Esfahani <dirty at apple.com> wrote:
>>> My first patch, so please be gentle...
>>>
>>> This fixes a problem I was seeing with the COFF back end.  All of the string data was getting put into one data fragment, and so none of the fixup references were correct.
>>
>> Can you post a test case so I can take a look? I'm not quite sure what
>> the problem is. But I know this isn't the best way to fix the problem.
>>
>
> Here's a reproducible case: http://pastebin.com/NCActA1d


Reduces to:

--------------------------------------------------------------
#include <stdio.h>

int main()
{
    printf("Hello ");
    printf("World!\n");
}
--------------------------------------------------------------

This will output: Hello Hello

>
> Build it with: clang -ccc-host-triple i686-pc-win32 -c -integrated-as -o HelloWorld.o HelloWorld.c.
>
> If you dump HelloWorld.o, you'll see that the symbol table entries for L_.str2, L_.str1 and L_.str have values of 0.
>
> Enclosed is, what I believe to be, a better fix for this problem.  Instead of allocating a new data fragment for each new label, just make sure to account for the symbol's offset within the fragment when creating the symbol table value.

Committed in r110104

>
> This patch is based on revision 110062.
>
> I also noticed that the comment for Code in MCInstFragment was incorrect.  I attempted to fix it, but it's probably horribly wrong...

Daniel will have to take a look at that.

>
>>> I also made sure the TimeDateStamp in the COFF header had a correct value.

Commited in r110101, but I forgot to add you as the author. I
apologize about that.

- Michael Spencer

>>
>> This is right.
>>
>>> I also fixed a spot in MCFragment where Offset wasn't being properly initialized.
>>>
>>> This patch was generated against revision 109821.
>>
>> Also, was this change required to get it to work? The SymbolTableIndex
>> should be set down on line 674.
>>
>> @@ -578,12 +580,13 @@ void WinCOFFObjectWriter::RecordRelocation(const
>> MCAssembler &Asm,
>>   FixedValue = Target.getConstant();
>>
>>   COFFRelocation Reloc;
>>
>>   Reloc.Data.VirtualAddress = Layout.getFragmentOffset(Fragment);
>>   Reloc.Symb = coff_symbol;
>> +  Reloc.Data.SymbolTableIndex = 0;
>>
>>   Reloc.Data.VirtualAddress += Fixup.getOffset();
>>
>>   switch (Fixup.getKind()) {
>>   case FirstTargetFixupKind: // reloc_pcrel_4byte
>>     Reloc.Data.Type = COFF::IMAGE_REL_I386_REL32;
>>
>> - Michael Spencer
>
>
> The Reloc.Data.SymbolTableIndex = 0 line was required because of this warning:
>
> /usr/include/c++/4.2.1/ext/new_allocator.h: In member function ‘virtual void<unnamed>::WinCOFFObjectWriter::RecordRelocation(const llvm::MCAssembler&, const llvm::MCAsmLayout&, const llvm::MCFragment*, const llvm::MCFixup&, llvm::MCValue, uint64_t&)’:
> /usr/include/c++/4.2.1/ext/new_allocator.h:107: warning: ‘Reloc.<unnamed>::COFFRelocation::Data.llvm::COFF::relocation::SymbolTableIndex’ is used uninitialized in this function
> WinCOFFObjectWriter.cpp:582: note: ‘Reloc.<unnamed>::COFFRelocation::Data.llvm::COFF::relocation::SymbolTableIndex’ was declared here
>
>
>
> Cameron Esfahani
> dirty at apple.com
>
> "You only live once, and the way I live, once is enough"
>
> Frank Sinatra
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>