dolik-rce / pegof

PEG grammar optimizer and formatter
Other
9 stars 2 forks source link

Conflict temporary file names #12

Closed masatake closed 2 months ago

masatake commented 2 months ago

I'm intergrating pegof to the build script of Universal Ctags (https://github.com/universal-ctags/ctags/pull/4026).

During integration, I found pegof processes suddenly died when I passed -j16 option to make. I inspected it. It seemed that pegof used current time with second resolution:

$ strace -kk -e mkdir -f ../pegof/build/pegof -O all -X unused-capture -o peg/kotlin.pego -i peg/kotlin.peg 
% strace -kk -e mkdir -f ../pegof/build/pegof -O all -X unused-capture -o peg/kotlin.pego -i peg/kotlin.peg
mkdir("/tmp/pegof_1719657733", 0777)    = 0
 > /usr/lib64/libc.so.6(mkdir+0xb) [0x10a2bb] ../sysdeps/unix/sysv/linux/mkdir.c:28
 > /usr/lib64/libstdc++.so.6.0.33(std::filesystem::create_directory(std::filesystem::__cxx11::path const&, std::error_code&)+0x1a) [0x19599a] ../../../../../libstdc++-v3/src/c++17/fs_ops.cc:582
 > /usr/lib64/libstdc++.so.6.0.33(std::filesystem::create_directory(std::filesystem::__cxx11::path const&)+0x2a) [0x19685a] ../../../../../libstdc++-v3/src/c++17/fs_ops.cc:604
 > /home/yamato/var/pegof/build/pegof(Checker::Checker()+0x1aa) [0x46e414]
 > /home/yamato/var/pegof/build/pegof(main+0x75) [0x4728cf]
 > /usr/lib64/libc.so.6(__libc_start_call_main+0x77) [0x2a087] ../sysdeps/nptl/libc_start_call_main.h:58
 > /usr/lib64/libc.so.6(__libc_start_main@@GLIBC_2.34+0x8a) [0x2a14a] ../csu/libc-start.c:360
 > /home/yamato/var/pegof/build/pegof(_start+0x24) [0x406bd4]

Using higher resolution, or mixing process id, or something technique is needed.

masatake commented 2 months ago
Checker::Checker() {
    fs::path tmp_dir = fs::temp_directory_path() / ("pegof_" + std::to_string(time(0)));
time(2)                       System Calls Manual                      time(2)

NAME
       time - get time in seconds

LIBRARY
       Standard C library (libc, -lc)

SYNOPSIS
       #include <time.h>

       time_t time(time_t *_Nullable tloc);

DESCRIPTION
       time()  returns  the  time  as  the  number of seconds since the Epoch,
       1970-01-01 00:00:00 +0000 (UTC).

Using mkstemp may be better.

       mkstemp()
              POSIX.1-2001.
masatake commented 2 months ago
diff --git a/src/checker.cc b/src/checker.cc
index 28c3a0a..e29f8fa 100644
--- a/src/checker.cc
+++ b/src/checker.cc
@@ -42,7 +42,8 @@ std::string Stats::compare(const Stats& s) const {
 }

 Checker::Checker() {
-    fs::path tmp_dir = fs::temp_directory_path() / ("pegof_" + std::to_string(time(0)));
+    fs::path tmp_dir = fs::temp_directory_path() / ("pegof_" + std::to_string(time(0))
+                           + "_" + std::to_string(getpid()));
     output = (tmp_dir / "output").native();
     tmp = tmp_dir.native();
     fs::create_directory(tmp_dir);

This works. However, the name of the directory is predictable.

dolik-rce commented 2 months ago

I've fixed it using mkdtemp, see #13.