This is the mail archive of the binutils@sourceware.org mailing list for the binutils 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] gas: fix a few omissions in .cfi_label handling


On Thu, Feb 5, 2015 at 8:28 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 05.02.15 at 17:21, <hjl.tools@gmail.com> wrote:
>> On Thu, Feb 5, 2015 at 8:17 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 05.02.15 at 16:50, <hjl.tools@gmail.com> wrote:
>>>> On Thu, Feb 5, 2015 at 7:45 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>> On 05.02.15 at 16:38, <hjl.tools@gmail.com> wrote:
>>>>>> On Thu, Feb 5, 2015 at 7:06 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> While actually starting to use that new directive, I noticed a few
>>>>>>> oversights of the original commit.
>>>>>>>
>>>>>>> gas/
>>>>>>> 2015-02-05  Jan Beulich  <jbeulich@suse.com>
>>>>>>>
>>>>>>>         * dw2gencfi.c (select_cie_for_fde): Also bail on CFI_label.
>>>>>>>         (cfi_change_reg_numbers): Also do nothing for CFI_label.
>>>>>>>         (cfi_pseudo_table): Also handle .cfi_label when not supporting
>>>>>>>         CFI directives.
>>>>>>>
>>>>>>
>>>>>> Please add some testcases.
>>>>>
>>>>> For what? The original patch added some already, and I really don't
>>>>> see the point of me wasting time on trying to figure out how I could
>>>>> demonstrate that things could go wrong without these (minor and
>>>>> obvious) adjustments.
>>>>
>>>> How does one know what the new directive should
>>>> behave?  Without the testcase, it may be broken and
>>>> we won't even know it.
>>>
>>> There is no new directive being added here. All that happens are a
>>> couple of adjustments to the code that added it recently (with - upon
>>> your request - a test case).
>>>
>>
>> Doesn't your new patch add cfi_label?  If it is just a dummy,
>> why not just replace those dummy CFI directives with a
>> cfi_dummy directive?
>
> It adds (fake) support for the directive in the case when CFI
> directives aren't supported (as said in the last log entry still
> quoted above). There are two tables, and it's only the latter
> (dummy) one that I forgot to update in the original patch.
>

Does your new patch change the assembler output in
any way?  If yes, please show it with a new testcase
or an update to existing ones.



-- 
H.J.


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