Skip to content

Commit

Permalink
Ensure that created log files' names start with the specified name
Browse files Browse the repository at this point in the history
Prior to this change, if suffixLogs (i.e. TR_EnablePIDExtension) was
enabled (which by default it is), log files for compilation threads
other than compilation thread 0 would be opened using a name consisting
of only the extra suffix, e.g. ".193310.88163.20211109.150243.193310".

The cause was as follows:

1. When openLogFile() got a valid idSuffix, it would concatenate
   _logFileName with it into a stack buffer (filename).

2. Then when suffixLogs was also enabled, it would use the same stack
   buffer (filename) as the destination for the concatenation of the
   name so far (e.g. foo.log.1) with the PID and time. This was a call
   to sprintf() in which the destination buffer was also passed as an
   argument corresponding to a %s format specifier. This could
   (depending on platform?) behave as though the string argument was
   just an empty string.

Note that the compilation thread ID was also lost, so any threads other
than compilation thread 0 could pass the same filename to trfopen() if
they started to trace within the same second, resulting in too few log
files created. Since trfopen() is implemented via fopen(), we can expect
that such a log opened multiple times will be truncated on each open
(after the first).

This problem is fixed by replacing the single buffer (filename) on the
stack with a pair of buffers (buf0, buf1). At any given time, one is
considered the next destination (destBuf). After formatting into
destBuf, the buffers are swapped to ensure that it is safe to format
into destBuf again. With this pair of buffers in place, it is no longer
necessary to declare another buffer (tmp) for getFormattedName().
  • Loading branch information
jdmpapin committed Nov 9, 2021
1 parent 79e5193 commit 1a1e980
Showing 1 changed file with 18 additions and 11 deletions.
29 changes: 18 additions & 11 deletions compiler/control/OMROptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3764,16 +3764,22 @@ void OMR::Options::openLogFile(int32_t idSuffix)
if (_suffixLogsFormat)
self()->setOption(TR_EnablePIDExtension);

char filename[1025];
#define FN_BUF_SIZE 1025
char buf0[FN_BUF_SIZE];
char buf1[FN_BUF_SIZE];
char *destBuf = buf0;
char *otherBuf = buf1;

char *fn = _logFileName;

if (idSuffix >= 0) // Must add the suffix to the name
{
size_t len = strlen(_logFileName);
if (len >= sizeof(filename) - 10) // int32_t is at most 10 decimal digits
if (len >= FN_BUF_SIZE - 10) // int32_t is at most 10 decimal digits
return; // may overflow the buffer
sprintf(filename, "%s.%d", _logFileName, idSuffix);
fn = filename;
sprintf(destBuf, "%s.%d", _logFileName, idSuffix);
fn = destBuf;
std::swap(destBuf, otherBuf);
}

char *fmodeString = "wb+";
Expand All @@ -3785,24 +3791,25 @@ void OMR::Options::openLogFile(int32_t idSuffix)
size_t len = strlen(fn);
char pid_buf[20];
char time_buf[20];
if (len >= sizeof(filename) - sizeof(pid_buf) - sizeof(time_buf))
if (len >= FN_BUF_SIZE - sizeof(pid_buf) - sizeof(time_buf))
return; // may overflow the buffer
getTRPID(pid_buf);
getTimeInSeconds(time_buf);
sprintf(filename, "%s.%s.%s", fn, pid_buf, time_buf);
fn = filename;
sprintf(destBuf, "%s.%s.%s", fn, pid_buf, time_buf);
fn = destBuf;
std::swap(destBuf, otherBuf);
}

char tmp[1025];
fn = _fe->getFormattedName(tmp, sizeof(tmp), fn, _suffixLogsFormat, true);
fn = _fe->getFormattedName(destBuf, FN_BUF_SIZE, fn, _suffixLogsFormat, true);
_logFile = trfopen(fn, fmodeString, false);
}
else
{
char tmp[1025];
fn = _fe->getFormattedName(tmp, sizeof(tmp), fn, NULL, false);
fn = _fe->getFormattedName(destBuf, FN_BUF_SIZE, fn, NULL, false);
_logFile = trfopen(fn, fmodeString, false);
}

#undef FN_BUF_SIZE
}

if (_logFile != NULL)
Expand Down

0 comments on commit 1a1e980

Please sign in to comment.