shadow-maint / shadow

Upstream shadow tree
Other
287 stars 227 forks source link

tests/unit/test_xasprintf.c: Fix use of volatile pointer #1033

Closed alejandro-colomar closed 2 weeks ago

alejandro-colomar commented 2 weeks ago

volatile needs to be casted away behind a [[gnu::noipa]] function, to make that invisible to the compiler. Otherwise, the compiler can see that it is being discarded, and is free to abuse Undefined Behavior.

Closes: https://github.com/shadow-maint/shadow/issues/1028 Reported-by: @zeha

alejandro-colomar commented 2 weeks ago

@zeha can you test this, please?

zeha commented 2 weeks ago

Can report two improvments: 1) build time warning is gone 2) test_xasprintf_exit now passes

However test_xasprintf_ok is still broken for me:

[==========] tests: Running 2 test(s).
[ RUN      ] test_xasprintf_exit
asprintf: Success
[       OK ] test_xasprintf_exit
[ RUN      ] test_xasprintf_ok
[  ERROR   ] --- %s() has remaining non-returned values.
: __wrap_vasprintftest_xasprintf.c:118: note: remaining item was declared here

I'd suggest merging it and then looking at test_xasprintf_ok later. I'll also be on a vacation for two weeks :)

alejandro-colomar commented 2 weeks ago

Reviewed and tested by @zeha

$ git range-diff master gh/xasprintf xasprintf 
1:  d9d2ecb3 = 1:  d9d2ecb3 tests/unit/test_xasprintf.c: Cosmetic
2:  596040f9 ! 2:  00c70a24 tests/unit/test_xasprintf.c: Fix use of volatile pointer
    @@ Commit message

         Closes: <https://github.com/shadow-maint/shadow/issues/1028>
         Reported-by: Chris Hofstaedtler <zeha@debian.org>
    +    Tested-by: Chris Hofstaedtler <zeha@debian.org>
    +    Reviewed-by: Chris Hofstaedtler <zeha@debian.org>
         Signed-off-by: Alejandro Colomar <alx@kernel.org>

      ## tests/unit/test_xasprintf.c ##
alejandro-colomar commented 2 weeks ago

On Fri, Jun 28, 2024 at 05:40:38AM GMT, Chris Hofstaedtler wrote:

Can report two improvments: 1) build time warning is gone 2) test_xasprintf_exit now passes

However test_xasprintf_ok is still broken for me:

[==========] tests: Running 2 test(s).
[ RUN      ] test_xasprintf_exit
asprintf: Success
[       OK ] test_xasprintf_exit
[ RUN      ] test_xasprintf_ok
[  ERROR   ] --- %s() has remaining non-returned values.
: __wrap_vasprintftest_xasprintf.c:118: note: remaining item was declared here

I'd suggest merging it and then looking at test_xasprintf_ok later.

Thanks. Serge, please also merge this when you find some time.

Have a lovely day! Alex

I'll also be on a vacation for two weeks :)

-- Reply to this email directly or view it on GitHub: https://github.com/shadow-maint/shadow/pull/1033#issuecomment-2196819291 You are receiving this because you authored the thread.

Message ID: @.***>

-- https://www.alejandro-colomar.es/

alejandro-colomar commented 2 weeks ago

Can report two improvments:

1. build time warning is gone

2. `test_xasprintf_exit` now passes

However test_xasprintf_ok is still broken for me:

[==========] tests: Running 2 test(s).
[ RUN      ] test_xasprintf_exit
asprintf: Success
[       OK ] test_xasprintf_exit
[ RUN      ] test_xasprintf_ok
[  ERROR   ] --- %s() has remaining non-returned values.
: __wrap_vasprintftest_xasprintf.c:118: note: remaining item was declared here

This seems to be a separate issue. Please open a separate bug report for it. We'll need the help of some cmocka developer, since I suspect where's the problem, but don't know how to fix it (I'm not even sure it's our problem).

I'd suggest merging it and then looking at test_xasprintf_ok later. I'll also be on a vacation for two weeks :)