3

I am trying to understand why the following code generates an "argument might be clobbered.." warning. Here's a minimal sample:

#include <unistd.h>

extern char ** environ;

int test_vfork(char **args, char **newEnviron)
{
    pid_t pid = vfork();
    if (pid == 0) {
        execve(args[0], args, (newEnviron != NULL)? newEnviron : environ);
    }

    return 0;
}

And this is the output (this is gcc 4.3.3):

$ gcc -O2 -Wclobbered -c test.c -o test.o
test.c: In function ‘test_vfork’:
test.c:5: warning: argument ‘newEnviron’ might be clobbered by ‘longjmp’ or ‘vfork’

Also I found out that the warning goes away if I replace the execve line with the following:

    if (newEnviron != NULL)
        execve(commandLine[0], commandLine, newEnviron);
    else
        execve(commandLine[0], commandLine, environ);

Not sure why gcc likes this better than the original. Can anyone shed some light?

Grodriguez
  • 21,501
  • 10
  • 63
  • 107
  • Clang 3.4 (on OS X) compiles it without complaint, even with `-Wall -Wextra`. You might try a (much) newer GCC. – John Zwinck Aug 12 '14 at 12:32
  • http://stackoverflow.com/questions/7721854/what-sense-do-these-clobbered-variable-warnings-make – Grady Player Aug 12 '14 at 12:39
  • @GradyPlayer: I checked that already but don't see how that applies to this case. The `newEnviron` argument is not being modified through the code. – Grodriguez Aug 12 '14 at 12:41
  • I don't think that it needs to be modified to get the warning, and I don't know the version without the ternary is OK with the compiler, I suspect that is a bug... but did you try decorating the function with `returns_twice`? – Grady Player Aug 12 '14 at 12:45
  • Code-analysis is really difficult. Be thankful GCC tries to warn you it's not sure your code is safe, even though I dearly hope there's no actual danger with that code. – Deduplicator Aug 12 '14 at 13:08

1 Answers1

2

From C99's longjmp description:

All accessible objects have values, and all other components of the abstract
machine218) have state, as of the time the longjmp function was called, except
that the values of objects of automatic storage duration that are local to the
function containing the invocation of the corresponding setjmp macro that do not
have volatile-qualified type and have been changed between the setjmp invocation
and longjmp call are indeterminate.

If you make newEnviron a volatile object, this paragraph indicates that newEnviron will not be clobbered by longjmp. The specification or implementation of vfork might have a similar caveat.

--EDIT--

Because you have optimizations enabled and because newEnviron is non-volatile and because it is not accessed after its use in the ternary conditional operator, one optimization the implementation could perform for your conditional operator would be to actually re-assign newEnviron with the value of environ. Something like:

execve(args[0], args, (newEnviron = newEnviron ? newEnviron : environ));

But we know from the manual for vfork that modifying most objects before the execve results in undefined behaviour.

args doesn't suffer from the same concern because there is no such conditional test.

Your if statement has more sequence points and besides that, might not be as easily recognized as an optimization opportunity. However, I'd hazard a guess that the sequence points play a strong role in the optimization.

The optimization, by the way, is that the storage for newEnviron is repurposed for the result of the conditional operator, meaning neither another register is used (if registers would normally be used) nor additional stack-space is required (for systems using stacks).

I'll bet that if you were able to convince gcc that you needed to access the value of newEnviron after the execve line, the optimization would not be possible and your warning would disappear.

But of course, using volatile is a simple solution, too.

Shao
  • 537
  • 3
  • 7
  • This does not provide an answer to the question. To critique or request clarification from an author, leave a comment below their post. – Davidmh Aug 15 '14 at 13:42
  • 2
    Davidmh: There you go. I was too late to avoid your down-vote. And "Can anyone shed some light" seemed like a vague enough question to answer with "make it volatile". Opinions differ. – Shao Aug 15 '14 at 13:55
  • if you think the quesion is unclear, flag it as such. The answer now is much better now. BTW, the downvote was not from me, but here you have my +1 :) – Davidmh Aug 15 '14 at 14:36
  • I dont see how the question is vague. I am asking why gcc is giving this warning for newEnviron (while it doesn't say anything about, for example, args) , and why there is no warning for the second version. – Grodriguez Aug 16 '14 at 18:03
  • Davidmh: Oh, ok. Thanks! :) Grodriguez: It wasn't clear to me from your questions (2) what level of detail you wanted... For the most detailed answer, we could delve into the source-code for GCC to find it. For a simple answer, some insight would be "to avoid the warning, try making it **volatile**." I've edited my answer with a more detailed guess. I hope it helps! – Shao Aug 17 '14 at 05:24