[PATCH v4] Fix failure when CFLAGS contains -DNDEBUG (Bug 25251)

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

[PATCH v4] Fix failure when CFLAGS contains -DNDEBUG (Bug 25251)

Carlos O'Donell-6
OK, here is v4
- Clarified all comments that this is about the build system in
  some way adding -DNDEBUG and that we want to undo that for the test.

8< --- 8< --- 8<
Building tests with -DNDEBUG in CFLAGS, gcc 9.2.1 issues the following error:
tst-assert-c++.cc: In function ‘int do_test()’:
tst-assert-c++.cc:66:12: error: unused variable ‘value’ [-Werror=unused-variable]
   66 |     no_int value;
      |            ^~~~~
tst-assert-c++.cc:71:18: error: unused variable ‘value’ [-Werror=unused-variable]
   71 |     bool_and_int value;
      |                  ^~~~~

The assert has been disabled by building glibc with CFLAGS, CXXFLAGS,
and CPPFLAGS with -DNDEBUG which removes the assert and leaves the
value unused.

We never want the assert disabled because that's the point of the
test, so we undefine NDEBUG before including assert.h to ensure that
we get assert correctly defined.
---
 assert/tst-assert-c++.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/assert/tst-assert-c++.cc b/assert/tst-assert-c++.cc
index 41cb487512..c01fc8bd25 100644
--- a/assert/tst-assert-c++.cc
+++ b/assert/tst-assert-c++.cc
@@ -16,6 +16,9 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
+/* Undefine NDEBUG to ensure the build system e.g. CFLAGS/CXXFLAGS
+   does not disable the asserts we want to test.  */
+#undef NDEBUG
 #include <assert.h>
 
 /* The C++ standard requires that if the assert argument is a constant
--
2.21.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v4] Fix failure when CFLAGS contains -DNDEBUG (Bug 25251)

Florian Weimer-5
* Carlos O'Donell:

> Building tests with -DNDEBUG in CFLAGS, gcc 9.2.1 issues the following error:
> tst-assert-c++.cc: In function ‘int do_test()’:
> tst-assert-c++.cc:66:12: error: unused variable ‘value’ [-Werror=unused-variable]
>    66 |     no_int value;
>       |            ^~~~~
> tst-assert-c++.cc:71:18: error: unused variable ‘value’ [-Werror=unused-variable]
>    71 |     bool_and_int value;
>       |                  ^~~~~
>
> The assert has been disabled by building glibc with CFLAGS, CXXFLAGS,
> and CPPFLAGS with -DNDEBUG which removes the assert and leaves the
> value unused.
>
> We never want the assert disabled because that's the point of the
> test, so we undefine NDEBUG before including assert.h to ensure that
> we get assert correctly defined.
> ---
>  assert/tst-assert-c++.cc | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/assert/tst-assert-c++.cc b/assert/tst-assert-c++.cc
> index 41cb487512..c01fc8bd25 100644
> --- a/assert/tst-assert-c++.cc
> +++ b/assert/tst-assert-c++.cc
> @@ -16,6 +16,9 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> +/* Undefine NDEBUG to ensure the build system e.g. CFLAGS/CXXFLAGS
> +   does not disable the asserts we want to test.  */
> +#undef NDEBUG
>  #include <assert.h>
>  
>  /* The C++ standard requires that if the assert argument is a constant

Looks good to me.

Thanks,
Florian

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v4] Fix failure when CFLAGS contains -DNDEBUG (Bug 25251)

Carlos O'Donell-6
On 12/10/19 11:57 AM, Florian Weimer wrote:

> * Carlos O'Donell:
>
>> Building tests with -DNDEBUG in CFLAGS, gcc 9.2.1 issues the following error:
>> tst-assert-c++.cc: In function ‘int do_test()’:
>> tst-assert-c++.cc:66:12: error: unused variable ‘value’ [-Werror=unused-variable]
>>    66 |     no_int value;
>>       |            ^~~~~
>> tst-assert-c++.cc:71:18: error: unused variable ‘value’ [-Werror=unused-variable]
>>    71 |     bool_and_int value;
>>       |                  ^~~~~
>>
>> The assert has been disabled by building glibc with CFLAGS, CXXFLAGS,
>> and CPPFLAGS with -DNDEBUG which removes the assert and leaves the
>> value unused.
>>
>> We never want the assert disabled because that's the point of the
>> test, so we undefine NDEBUG before including assert.h to ensure that
>> we get assert correctly defined.
>> ---
>>  assert/tst-assert-c++.cc | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/assert/tst-assert-c++.cc b/assert/tst-assert-c++.cc
>> index 41cb487512..c01fc8bd25 100644
>> --- a/assert/tst-assert-c++.cc
>> +++ b/assert/tst-assert-c++.cc
>> @@ -16,6 +16,9 @@
>>     License along with the GNU C Library; if not, see
>>     <https://www.gnu.org/licenses/>.  */
>>  
>> +/* Undefine NDEBUG to ensure the build system e.g. CFLAGS/CXXFLAGS
>> +   does not disable the asserts we want to test.  */
>> +#undef NDEBUG
>>  #include <assert.h>
>>  
>>  /* The C++ standard requires that if the assert argument is a constant
>
> Looks good to me.

Thanks. Pushed.

--
Cheers,
Carlos.