Clang violates the mkstemp() contract?

Hello,

My apologies for the half-researched problem, but unfortunately I don't have the time to verify this properly, and this looks like a subtle bug that might bite other people.

If on Minix I run clang on two source files:

    clang -v a.c b.c

compilation fails with the following output (The 'entering ...' and 'Created ...' are from debugging code I added to Path::makeUnique()):

Hello,

My apologies for the half-researched problem, but unfortunately I
don't have the time to verify this properly, and this looks like a
subtle bug that might bite other people.

If on Minix I run clang on two source files:

    clang -v a.c b.c

compilation fails with the following output (The 'entering ...' and
'Created ...' are from debugging code I added to Path::makeUnique()):

---------------
clang version 2.8 (trunk 108237)
Target: i386-pc-minix
Thread model: posix
entering HAVE_MKSTEMP section
Created mktemp temp filename '/tmp/cc-002440'
entering HAVE_MKSTEMP section
Created mktemp temp filename '/tmp/cc-002440'
entering HAVE_MKSTEMP section
Created mktemp temp filename '/tmp/cc-002440'
entering HAVE_MKSTEMP section
Created mktemp temp filename '/tmp/cc-002440'
  "/usr/llvm/bin/clang" -cc1 -triple i386-pc-minix -S -disable-free
-main-file-name a.c -mrelocation-model static -mdisable-fp-elim
-mconstructor-aliases -target-cpu pentium4 -v
-resource-dir /lib/clang/2.8 -ferror-limit 19 -fmessage-length 0
-fgnu-runtime -fdiagnostics-show-option -o /tmp/cc-002440.s -x c a.c
clang -cc1 version 2.8 based upon llvm 2.8svn hosted on i386-pc-minix
ignoring nonexistent directory "/lib/clang/2.8/include" #include
"..." search starts here: #include <...> search starts
here: /usr/local/include /usr/include
End of search list.
  "/usr/gnu/bin/gas" -o /tmp/cc-002440.o /tmp/cc-002440.s
  "/usr/llvm/bin/clang" -cc1 -triple i386-pc-minix -S -disable-free
-main-file-name b.c -mrelocation-model static -mdisable-fp-elim
-mconstructor-aliases -target-cpu pentium4 -v
-resource-dir /lib/clang/2.8 -ferror-limit 19 -fmessage-length 0
-fgnu-runtime -fdiagnostics-show-option -o /tmp/cc-002440.s -x c b.c
clang -cc1 version 2.8 based upon llvm 2.8svn hosted on i386-pc-minix
ignoring nonexistent directory "/lib/clang/2.8/include" #include
"..." search starts here: #include <...> search starts
here: /usr/local/include /usr/include
End of search list.
  "/usr/gnu/bin/gas" -o /tmp/cc-002440.o /tmp/cc-002440.s
  "/usr/gnu/bin/gld" -o
a.out /usr/gnu/lib/crtso.o /tmp/cc-002440.o /tmp/cc-002440.o -lc
-lgcc -L/usr/gnu/lib
-L/usr/gnu/lib/gcc/i686-pc-minix/4.4.3 /usr/gnu/lib/libend.a /tmp/cc-002440.o:/tmp/cc-002440.o:(.text+0x0):
multiple definition of
`main' /tmp/cc-002440.o:/tmp/cc-002440.o:(.text+0x0): first defined
here /tmp/cc-002440.o:/tmp/cc-002440.o:(.text+0x28): undefined
reference to `fA' /tmp/cc-002440.o:/tmp/cc-002440.o:(.text+0x28):
undefined reference to `fA' error: linker command failed with exit
code 1 (use -v to see invocation) ---------------

The compilation fails because for the compilation of both a.c and b.c
the same temporary files are used, meaning that the .o file of a.c is
overwritten when the .o file for b.c is generated, and the loader
simply gets the same .o file twice.

As expected, this compilation succeeds if -save-temps is added.

After some digging around I strongly suspect that someone has
overlooked the fact that mktemp() and mkstemp() may well return the
same filename twice. If you remove a file that was created by
mkstemp(), or if you never create the file that mas generated for you
by mktemp(), these functions may well return the same filename the
next time they are invoked with the same template. In fact, in
Driver::GetTemporaryPath() I see the code:

   // FIXME: Grumble, makeUnique sometimes leaves the file around!?
PR3837. P.eraseFromDisk(false, 0);

The file should probably not be removed at all. If there is concern
that the file stays there after process exit, it should be added to
removeFileOnSignal.

Which seems to confirm that an unwarranted assumption is made.

The reason this works on many platforms is because there the unique
filename is generated by first attempting to use a name based on a
random number. If a file with that name exists, the name is then
modified to avoid existing names. However, on Minix the first attempt
uses the process id, so you'll always get the same temporary filename
if the previous ones do not exist (any more), on other platforms that
is far less likely (but not impossible).

Sounds like mkstemp() on Minix is fairly predictable then.

Best regards,
--Edwin

The file should probably not be removed at all. If there is concern
that the file stays there after process exit, it should be added to
removeFileOnSignal.

Well, clang is actually not interested in that particular file, because it wants to append a .o or .s after the filename that was generated. That makes the cleanup a bit trickier.

Also, clang uses mktemp() if mkstemp() is not available on a particular platform, and in that case clang should actively create the file as soon as it gets the name.

Which seems to confirm that an unwarranted assumption is made.

The reason this works on many platforms is because there the unique
filename is generated by first attempting to use a name based on a
random number. If a file with that name exists, the name is then
modified to avoid existing names. However, on Minix the first attempt
uses the process id, so you'll always get the same temporary filename
if the previous ones do not exist (any more), on other platforms that
is far less likely (but not impossible).

Sounds like mkstemp() on Minix is fairly predictable then.

That's correct, but if you're implying that that is a security concern: AFAIK mkstemp() is supposed to be an atomic operation and therefore predictability is not a concern.