* PING: PATCH: PR other/48007: Unwind library doesn't work with UNITS_PER_WORD > sizeof (void *)
@ 2011-04-26 13:32 H.J. Lu
2011-05-18 17:05 ` H.J. Lu
0 siblings, 1 reply; 3+ messages in thread
From: H.J. Lu @ 2011-04-26 13:32 UTC (permalink / raw)
To: Ulrich Weigand
Cc: Richard Henderson, Jakub Jelinek, Ian Lance Taylor,
Andrew Pinski, GCC Patches
On Sat, Apr 9, 2011 at 6:52 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Mar 24, 2011 at 12:15 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Wed, Mar 23, 2011 at 12:22 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
>>> Richard Henderson wrote:
>>>> Because, really, if we consider the structure truly public, we can't even
>>>> change the number of registers for a given port to support new features of
>>>> the cpu.
>>>
>>> Indeed, and I remember we got bitten by that a long time ago, which is why
>>> s390.h now has this comment:
>>>
>>> /* Number of hardware registers that go into the DWARF-2 unwind info.
>>> To avoid ABI incompatibility, this number must not change even as
>>> 'fake' hard registers are added or removed. */
>>> #define DWARF_FRAME_REGISTERS 34
>>>
>>>> I don't suppose there's any way that we can declare these old
>>>> programs Just Broken, and drop this compatibility stuff?
>>>
>>> I wouldn't like that ... we did run into this problem in the wild, and
>>> some s390 users really run very old programs for some reason.
>>>
>>> However, I'm wondering: this bug that leaked the implementation of
>>> _Unwind_Context only ever affected the *original* version of the
>>> structure -- it was fixed before the extended context was ever
>>> added, right?
>>>
>>> If this is true, we'd still need to keep the original context format
>>> unchanged, but we'd be free to modify the *extended* format at any
>>> time, without ABI considerations and need for further versioning ...
>>>
>>
>> From what I can tell, the issues are:
>>
>> 1. _Unwind_Context is supposed to be opaque and we are free to
>> change it. We should be able to extend DWARF_FRAME_REGISTERS
>> to support the new hard registers if needed, without breaking binary
>> compatibility.
>> 2. _Unwind_Context was leaked and wasn't really opaque. To
>> provide backward binary compatibility, we are stuck with what we
>> had.
>>
>> Is that possible to implement something along the line:
>>
>> 1. Add some bits to _Unwind_Context so that we can detect
>> the leaked _Unwind_Context.
>> 2. When a leaked _Unwind_Context is detected at run-time,
>> as a compile time option, a target can either provide binary
>> compatibility or issue a run-time error.
>
> This is the attempt to implement it. Any comments?
>
> Thanks.
>
> --
> H.J.
> --
> 2011-04-09 H.J. Lu <hongjiu.lu@intel.com>
>
> PR other/48007
> * unwind-dw2.c (UNIQUE_UNWIND_CONTEXT): New.
> (_Unwind_Context): If UNIQUE_UNWIND_CONTEXT is defined, add
> dwarf_reg_size_table and value, remove version and by_value.
> (EXTENDED_CONTEXT_BIT): Don't define if UNIQUE_UNWIND_CONTEXT
> is defined.
> (_Unwind_IsExtendedContext): Likewise.
> (_Unwind_GetGR): Support UNIQUE_UNWIND_CONTEXT.
> (_Unwind_SetGR): Likewise.
> (_Unwind_GetGRPtr): Likewise.
> (_Unwind_SetGRPtr): Likewise.
> (_Unwind_SetGRValue): Likewise.
> (_Unwind_GRByValue): Likewise.
> (__frame_state_for): Initialize dwarf_reg_size_table field if
> UNIQUE_UNWIND_CONTEXT is defined.
> (uw_install_context_1): Likewise. Support UNIQUE_UNWIND_CONTEXT.
>
PING.
--
H.J.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PING: PATCH: PR other/48007: Unwind library doesn't work with UNITS_PER_WORD > sizeof (void *)
2011-04-26 13:32 PING: PATCH: PR other/48007: Unwind library doesn't work with UNITS_PER_WORD > sizeof (void *) H.J. Lu
@ 2011-05-18 17:05 ` H.J. Lu
0 siblings, 0 replies; 3+ messages in thread
From: H.J. Lu @ 2011-05-18 17:05 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
On Tue, Apr 26, 2011 at 6:07 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Sat, Apr 9, 2011 at 6:52 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Thu, Mar 24, 2011 at 12:15 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>> On Wed, Mar 23, 2011 at 12:22 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
>>>> Richard Henderson wrote:
>>>>> Because, really, if we consider the structure truly public, we can't even
>>>>> change the number of registers for a given port to support new features of
>>>>> the cpu.
>>>>
>>>> Indeed, and I remember we got bitten by that a long time ago, which is why
>>>> s390.h now has this comment:
>>>>
>>>> /* Number of hardware registers that go into the DWARF-2 unwind info.
>>>> To avoid ABI incompatibility, this number must not change even as
>>>> 'fake' hard registers are added or removed. */
>>>> #define DWARF_FRAME_REGISTERS 34
>>>>
>>>>> I don't suppose there's any way that we can declare these old
>>>>> programs Just Broken, and drop this compatibility stuff?
>>>>
>>>> I wouldn't like that ... we did run into this problem in the wild, and
>>>> some s390 users really run very old programs for some reason.
>>>>
>>>> However, I'm wondering: this bug that leaked the implementation of
>>>> _Unwind_Context only ever affected the *original* version of the
>>>> structure -- it was fixed before the extended context was ever
>>>> added, right?
>>>>
>>>> If this is true, we'd still need to keep the original context format
>>>> unchanged, but we'd be free to modify the *extended* format at any
>>>> time, without ABI considerations and need for further versioning ...
>>>>
>>>
>>> From what I can tell, the issues are:
>>>
>>> 1. _Unwind_Context is supposed to be opaque and we are free to
>>> change it. We should be able to extend DWARF_FRAME_REGISTERS
>>> to support the new hard registers if needed, without breaking binary
>>> compatibility.
>>> 2. _Unwind_Context was leaked and wasn't really opaque. To
>>> provide backward binary compatibility, we are stuck with what we
>>> had.
>>>
>>> Is that possible to implement something along the line:
>>>
>>> 1. Add some bits to _Unwind_Context so that we can detect
>>> the leaked _Unwind_Context.
>>> 2. When a leaked _Unwind_Context is detected at run-time,
>>> as a compile time option, a target can either provide binary
>>> compatibility or issue a run-time error.
>>
>> This is the attempt to implement it. Any comments?
>>
>> Thanks.
>>
>> --
>> H.J.
>> --
>> 2011-04-09 H.J. Lu <hongjiu.lu@intel.com>
>>
>> PR other/48007
>> * unwind-dw2.c (UNIQUE_UNWIND_CONTEXT): New.
>> (_Unwind_Context): If UNIQUE_UNWIND_CONTEXT is defined, add
>> dwarf_reg_size_table and value, remove version and by_value.
>> (EXTENDED_CONTEXT_BIT): Don't define if UNIQUE_UNWIND_CONTEXT
>> is defined.
>> (_Unwind_IsExtendedContext): Likewise.
>> (_Unwind_GetGR): Support UNIQUE_UNWIND_CONTEXT.
>> (_Unwind_SetGR): Likewise.
>> (_Unwind_GetGRPtr): Likewise.
>> (_Unwind_SetGRPtr): Likewise.
>> (_Unwind_SetGRValue): Likewise.
>> (_Unwind_GRByValue): Likewise.
>> (__frame_state_for): Initialize dwarf_reg_size_table field if
>> UNIQUE_UNWIND_CONTEXT is defined.
>> (uw_install_context_1): Likewise. Support UNIQUE_UNWIND_CONTEXT.
>>
>
> PING.
>
Hi Jason,
Can you take a look at:
http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00695.html
Thanks.
--
H.J.
^ permalink raw reply [flat|nested] 3+ messages in thread
* PING: PATCH: PR other/48007: Unwind library doesn't work with UNITS_PER_WORD > sizeof (void *)
@ 2011-03-21 18:37 H.J. Lu
0 siblings, 0 replies; 3+ messages in thread
From: H.J. Lu @ 2011-03-21 18:37 UTC (permalink / raw)
To: Ian Lance Taylor; +Cc: GCC Patches
On Mon, Mar 14, 2011 at 10:49 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>>
>>>>>>> We shouldn't save call frame hard registers as "void *". This patch
>>>>>>> changes the unwind library to save call frame hard registers as
>>>>>>> _Unwind_Word. OK for 4.7?
>>>>>>
>>>>>> I think this will break the ABI for the MIPS N32 ABI. Not to mention
>>>>>> the MIPS N32 ABI works fine with the unwinding part this way. Does
>>>>>> someone use the unwinding library to look at the registers in previous
>>>>>> stack frames?
>>>>>
>>>>> It may be psABI/implementation specific. X32 glibc force unwind calls
>>>>> _Unwind_SetGRValue to get a 64bit register value.
>>>>
>>>> So fix it on that side?
>>>>
>>>
>>> How? One function in question is
>>>
>>> /* Overwrite the saved value for register INDEX in CONTEXT with VAL. */
>>>
>>> static inline void
>>> _Unwind_SetGRValue (struct _Unwind_Context *context, int index,
>>> _Unwind_Word val)
>>>
>>>
>>> Are you saying it shouldn't be called if UNITS_PER_WORD > sizeof (void *)?
>>>
>>
>> FWIW, the type of GR is _Unwind_Word, not void *. They may not
>> have the same size. Why does the DWARF unwind library use
>> void * to store GR? Can a target have an option to save a _Unwind_Word
>> value in _Unwind_Word, instead of void *?
>>
>
> From what I can tell, _Unwind_Context is internal to the unwind library
> and only one copy unwind library can be used in a process. I don't
> think changing _Unwind_Context will impact binary compatibility.
>
> Is my patch OK for 4.7?
>
PING:
http://gcc.gnu.org/ml/gcc-patches/2011-03/msg00267.html
--
H.J.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-05-18 14:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-26 13:32 PING: PATCH: PR other/48007: Unwind library doesn't work with UNITS_PER_WORD > sizeof (void *) H.J. Lu
2011-05-18 17:05 ` H.J. Lu
-- strict thread matches above, loose matches on Subject: below --
2011-03-21 18:37 H.J. Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).