On 02/26/2018 02:20 AM, Jakub Jelinek wrote:
> On Sun, Feb 25, 2018 at 05:56:28PM -0600, Daniel Santos wrote:
>>> --- libgcc/config/i386/i386-asm.h.jj 2018-01-03 10:42:56.317763517
>>> +0100
>>> +++ libgcc/config/i386/i386-asm.h 2018-02-22 15:33:43.812922298 +0100
>>> @@ -27,8 +27,47 @@ see the files COPYING3 and COPYING.RUNTI
>>> #define I386_ASM_H
>>>
>>> #include "auto-target.h"
>>> +#undef PACKAGE_VERSION
>>> +#undef PACKAGE_NAME
>>> +#undef PACKAGE_STRING
>>> +#undef PACKAGE_TARNAME
>>> +#undef PACKAGE_URL
>> This is a beautiful, temporary(?) fix to an ugly problem!
>>
>>> #include "auto-host.h"
>>> --- libgcc/config/i386/cygwin.S.jj 2018-01-03 10:42:56.309763515 +0100
>>> +++ libgcc/config/i386/cygwin.S 2018-02-22 15:30:34.597925496 +0100
>>> @@ -23,31 +23,13 @@
>>> * <http://www.gnu.org/licenses/>.
>>> */
>>>
>>> -#include "auto-host.h"
>> The following include should be here.
>>
>> +#include "i386-asm.h"
> I don't understand this. i386-asm.h needs (both before my patch and after
> it) both auto-host.h and auto-target.h, as it tests
> HAVE_GAS_SECTIONS_DIRECTIVE (this one newly, comes from cygwin.S)
The problem is that HAVE_GAS_SECTIONS_DIRECTIVE gets defined (or not) in
../../gcc/auto-host.h, but you are testing it before including
auto-host.h, either directly or via i386-asm.h. So if i386-asm.h
depends upon HAVE_GAS_SECTIONS_DIRECTIVE first being defined then it is
a circular dependency.
In its current form, cygwin.S would never define USE_GAS_CFI_DIRECTIVES
prior to including i386-asm.h and also never emit
.cfi_sections .debug_frame
and rather or not USE_GAS_CFI_DIRECTIVES ends up being defined to 1 or 0
depends upon the test of __GCC_HAVE_DWARF2_CFI_ASM in i386-asm.h.
So this area is new for me, but I don't understand why we're testing
HAVE_GAS_SECTIONS_DIRECTIVE in cygwin.S and __GCC_HAVE_DWARF2_CFI_ASM
when included from one of the stubs. Is this an error, or a lack of my
understanding or both? :)
> HAVE_GAS_HIDDEN
> macros defined in auto-host.h
> and
> HAVE_AS_AVX
> macro defined in auto-target.h.
> Including auto-host.h when i386-asm.h will include it again just doesn't
> work, these headers don't have multiple inclusion guards. And only including
> auto-target.h will work only if the
> .hidden
> and
> .cfi_sections .debug_frame
> tests are duplicated from gcc/configure.ac to libgcc/configure.ac, then we
> could include just auto-target.h in i386-asm.h.
> I've just followed what i386-asm.h has been doing.
And it's possible that I failed to test something correctly before
presuming it to be available, although I *think* the test for .hidden is
good.
>
> Jakub
>
Thanks for your work on this. If we need to test for CFI directives
differently when being included from cygwin.S, maybe we can just define
a simple cpp macro to indicate this and let i386-asm.h encapsulate the
implementation of it (e.g., testing HAVE_GAS_SECTIONS_DIRECTIVE or
__GCC_HAVE_DWARF2_CFI_ASM as appropriate).
Ultimately, the proper cleanup will be moving these tests out of
{gcc,libgcc}/configure.ac and into .m4 files in the root config
directory so that we don't uglify them with massive copy & pastes.
These tests are also fairly complex as there are a lot of dependencies.
m4 isn't my strong suite, but I can look at this after we're out of code
freeze.
Daniel
