[llvm-commits] [llvm] r97183 - /llvm/trunk/lib/CodeGen/AsmPrinter/DwarfException.cpp

[Bill Wendling]

On Feb 26, 2010, at 2:09 AM, Duncan Sands wrote:

> Hi Bill,
> 
>> Catch a corner case where adding the padding to the "TType base offset" field
>> will eliminate the need for padding in the "Call site table length". E.g., if
>> we have this:
>> 
>>     GCC_except_table1:
>>     Lexception1:
>>         .byte   0xff  ## @LPStart Encoding = omit
>>         .byte   0x9b  ## @TType Encoding = indirect pcrel sdata4
>>         .byte   0x7f  ## @TType base offset
>>         .byte   0x03  ## Call site Encoding = udata4
>>         .byte   0x89  ## Call site table length
>> 
>> with padding of 1. We want to emit the padding like this:
>> 
>>     GCC_except_table1:
>>     Lexception1:
>>         .byte   0xff  ## @LPStart Encoding = omit
>>         .byte   0x9b  ## @TType Encoding = indirect pcrel sdata4
>>         .byte   0xff  ## @TType base offset
>>         .space  1,0   ## Padding
>>         .byte   0x03  ## Call site Encoding = udata4
>>         .byte   0x89  ## Call site table length
>> 
>> and not with padding on the "Call site table length" entry.
> 
> aren't you back into the GCC style "oops, a value with variable length encoding
> changed, needed a different number of bytes to output, this shifted the start
> of the table, need to recalculate the alignment" loop problem?
> By putting padding inside "Call site table length", you change the value of
> "TType base offset".  Since this is uleb128 encoded, that can change the length
> of "TType base offset", which shifts the position of the ttypes, so changes the
> value you need for alignment, so changes the number of bytes for "Call site
> table length", so changes the value of "TType base offset", so changes the
> length of "TType base offset" etc.  Rinse and repeat.  If this is possible, then
> I'm afraid your patch doesn't solve the problem, just one instance of it.
> 
> To avoid such issues, maybe you can put the alignment padding inside "TType base
> offset ", rather than in "Call site table length"?  Since "TType base offset" is
> the first field to use a variable length encoding, a bunch of problems should go
> away.
> 
I would love to assign it to TType base offset all of the time, but it's not guaranteed to be there. In several followup patches, I believe I've solved the problem of "rinse & repeat". But without needing a loop. :) Here's what my patch does:

If we output the "TType base offset" (TBO), then we put the padding in there. This won't affect the "Call site table length" (CSTL) because that won't change.

If we don't emit the TBO, then all of the alignment goes into the CSTL.

I see an error in my reasoning. All of the tricky stuff that I do with the TBO isn't necessary. We should just emit the padding there if available and not in the CSTL. No need for fancy adding to the offset, etc.

-bw