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

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Jyotsna Verma
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
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

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

Richard Smith
On Wed, Apr 17, 2013 at 10:39 AM, Jyotsna Verma <[hidden email]> wrote:
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.

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.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

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

Jyotsna Verma

>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

 

--

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Richard Smith
Sent: Wednesday, April 17, 2013 2:19 PM
To: Jyotsna Verma
Cc: Daniel Dunbar; [hidden email] Developers
Subject: Re: c11.c/cxx11.cpp tests are failing for Hexagon

 

On Wed, Apr 17, 2013 at 10:39 AM, Jyotsna Verma <[hidden email]> wrote:

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.

 

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.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

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

Richard Smith
On Thu, Apr 18, 2013 at 10:16 AM, Jyotsna Verma <[hidden email]> wrote:

>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,

Jyotsna

 

--

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Richard Smith
Sent: Wednesday, April 17, 2013 2:19 PM
To: Jyotsna Verma
Cc: Daniel Dunbar; [hidden email] Developers
Subject: Re: c11.c/cxx11.cpp tests are failing for Hexagon

 

On Wed, Apr 17, 2013 at 10:39 AM, Jyotsna Verma <[hidden email]> wrote:

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.

 

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.



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

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

Daniel Dunbar
In reply to this post by Richard Smith
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:

Is that not the case?

 - Daniel


On Wed, Apr 17, 2013 at 12:19 PM, Richard Smith <[hidden email]> wrote:
On Wed, Apr 17, 2013 at 10:39 AM, Jyotsna Verma <[hidden email]> wrote:
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.

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.


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

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

Jyotsna Verma

Thanks Daniel! It works now.

 

-Jyotsna

--

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

 

From: [hidden email] [mailto:[hidden email]] On Behalf Of Daniel Dunbar
Sent: Thursday, April 18, 2013 12:55 PM
To: Richard Smith
Cc: Jyotsna Verma; [hidden email] Developers
Subject: Re: c11.c/cxx11.cpp tests are failing for Hexagon

 

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:

 

Is that not the case?

 

 - Daniel

 

On Wed, Apr 17, 2013 at 12:19 PM, Richard Smith <[hidden email]> wrote:

On Wed, Apr 17, 2013 at 10:39 AM, Jyotsna Verma <[hidden email]> wrote:

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.

 

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.

 


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev