c11.c/cxx11.cpp tests are failing for Hexagon

Hi Daniel/Richard,

I'm seeing c11.c and cxx11.cpp tests failing on Hexagon with these two
changes - r179427 and r179419.

After your changes, we include stdint.h file (from lib/Headers) in both
these tests. However, if __STDC_HOSTED__ macro is defined,
lib/Headers/stdint.h includes system stdint.h file which includes features.h
-> gnu/stubs-32.h. Since, we don't have stubs-32.h header file available for
Hexagon, it causes the tests to fail.

Please help me understand why the tests were modified to include stdint.h
file and how to fix them for Hexagon.

Thanks,
Jyotsna

These tests are testing the behavior of our <stdint.h> implementation. It
would certainly make sense to make them avoid including a system-provided
<stdint.h>. That will probably require somewhat different fixes for each
test:

I think (perhaps Daniel can confirm) that the c11.c test just wants to test
the contents of our own <stdint.h>, so modifying the test's RUN: line so
that the system-provided <stdint.h> is not on the search path would suffice.

For the cxx11.c test, what we're checking is that our builtin <stdint.h>
correctly works around http://sourceware.org/PR15366 -- though a better
(non-system-dependent) test here would be to craft a "system header"
<stdint.h> with the C99-specified behavior and put that in the system
headers include path for the test.

These tests are testing the behavior of our <stdint.h> implementation. It would certainly make sense to make them avoid including a system-provided <stdint.h>. That will probably require somewhat different fixes for each test:

I think (perhaps Daniel can confirm) that the c11.c test just wants to test the contents of our own <stdint.h>, so modifying the test’s RUN: line so that the system-provided <stdint.h> is not on the search path would suffice.

I don’t think changing the search path alone will help. As I mentioned earlier, system-provided stdint.h is included from lib/Headers/stdint.h if STDC_HOSTED macro is defined. I found this piece of code in lib/Frontend/InitPreprocessor.cpp which defines the macro:

if (LangOpts.Freestanding)

Builder.defineMacro(“STDC_HOSTED”, “0”);

else

Builder.defineMacro(“STDC_HOSTED”);

I think the only way to avoid system stdint.h from being included is to specify –ffreestanding option at the run command.

For the cxx11.c test, what we’re checking is that our builtin <stdint.h> correctly works around http://sourceware.org/PR15366 – though a better (non-system-dependent) test here would be to craft a “system header” <stdint.h> with the C99-specified behavior and put that in the system headers include path for the test.

I’m not sure if I understand it correctly. <stdint.h> in lib/Headers defines __STDC_LIMIT_MACROS and  __STDC_CONSTANT_MACROS which are used by system <stdint.h> to define SIZE_MAX which is checked in the test. So, I suppose you want to craft the new header file with these sections. But, what do you mean by putting the file in the system headers include path? Are you proposing to modify the RUN: line to search some temporary location?

I’m planning to XFAIL these two tests for Hexagon until the fix.

Thanks,

Jyotsna

>These tests are testing the behavior of our <stdint.h> implementation.
It would certainly make sense to make them avoid including a
system-provided <stdint.h>. That will probably require somewhat different
fixes for each test:****

** **

>I think (perhaps Daniel can confirm) that the c11.c test just wants to
test the contents of our own <stdint.h>, so modifying the test's RUN: line
so that the system-provided <stdint.h> is not on the search path would
suffice.****

** **

I don’t think changing the search path alone will help. As I mentioned
earlier, system-provided stdint.h is included from lib/Headers/stdint.h if
__STDC_HOSTED__ macro is defined.

It's also conditional on __has_include_next(<stdint.h>), and changing the
search path to remove the system headers would make that false.

I found this piece of code in lib/Frontend/InitPreprocessor.cpp which
defines the macro:****

** **

  if (LangOpts.Freestanding)****

    Builder.defineMacro("__STDC_HOSTED__", "0");****

  else****

    Builder.defineMacro("__STDC_HOSTED__");****

** **

I think the only way to avoid system stdint.h from being included is to
specify –ffreestanding option at the run command.

Sure, that's another option, and would be fine by me.

>For the cxx11.c test, what we're checking is that our builtin <stdint.h>
correctly works around http://sourceware.org/PR15366 -- though a better
(non-system-dependent) test here would be to craft a "system header"
<stdint.h> with the C99-specified behavior and put that in the system
headers include path for the test.****

** **

I’m not sure if I understand it correctly. <stdint.h> in lib/Headers defines __STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS which are used by system <stdint.h> to define SIZE_MAX which is checked in the test. So, I suppose you want to craft the new header file with these sections. But, what do you mean by putting the file in the system headers include path? Are you proposing to modify the RUN: line to search some temporary location?

Exactly, yes. We set up some fake system headers area which contains a

stdint.h, and put something inside it which verifies that we have defined
those macros before including it.

I’m planning to XFAIL these two tests for Hexagon until the fix.

OK.

Thanks,****

Hi Jyotsna,

You are right this is poor behavior in the tests (which shouldn’t ever depend on the system headers).

I believe the c11.c test should be fixed here:
http://llvm.org/viewvc/llvm-project?view=revision&revision=179723

Is that not the case?

  • Daniel

Thanks Daniel! It works now.

-Jyotsna