Page MenuHomePhabricator

Save strings for CC_PRINT env vars
ClosedPublic

Authored by SeanP on Mar 12 2021, 1:52 PM.

Details

Summary

The contents of the string returned by getenv() is not guaranteed across calls to getenv(). The code to handle the CC_PRINT etc env vars calls getenv() and saves the results in just a char *. The string returned by getenv() needs to be copied and saved. Switching the type of the strings from char * to std::string will do this and manage the alloated memory.

Diff Detail

Unit TestsFailed

TimeTest
480 msx64 debian > Clang.Driver::cc-print-proc-stat.c
Script: -- : 'RUN: at line 1'; env CC_PRINT_PROC_STAT=1 CC_PRINT_PROC_STAT_FILE=/mnt/disks/ssd0/agent/llvm-project/build/tools/clang/test/Driver/Output/cc-print-proc-stat.c.tmp.csv /mnt/disks/ssd0/agent/llvm-project/build/bin/clang -no-canonical-prefixes -S -o /mnt/disks/ssd0/agent/llvm-project/build/tools/clang/test/Driver/Output/cc-print-proc-stat.c.tmp.s /mnt/disks/ssd0/agent/llvm-project/clang/test/Driver/cc-print-proc-stat.c
500 msx64 debian > Clang.Driver::crash-report-with-asserts.c
Script: -- : 'RUN: at line 1'; rm -rf /mnt/disks/ssd0/agent/llvm-project/build/tools/clang/test/Driver/Output/crash-report-with-asserts.c.tmp
450 msx64 debian > Clang.Driver::crash-report.c
Script: -- : 'RUN: at line 1'; rm -rf /mnt/disks/ssd0/agent/llvm-project/build/tools/clang/test/Driver/Output/crash-report.c.tmp
490 msx64 debian > Clang.Preprocessor::headermap-rel2.c
Script: -- : 'RUN: at line 1'; rm -f /mnt/disks/ssd0/agent/llvm-project/build/tools/clang/test/Preprocessor/Output/headermap-rel2.c.tmp.hmap
40 msx64 debian > Flang.Semantics::omp-do05.f90
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/flang/test/Semantics/test_errors.sh /mnt/disks/ssd0/agent/llvm-project/flang/test/Semantics/omp-do05.f90 /mnt/disks/ssd0/agent/llvm-project/build/tools/flang/test/Semantics/Output/omp-do05.f90.tmp /mnt/disks/ssd0/agent/llvm-project/build/bin/f18 -intrinsic-module-directory /mnt/disks/ssd0/agent/llvm-project/build/tools/flang/include/flang -fopenmp
View Full Test Results (12 Failed)

Event Timeline

SeanP requested review of this revision.Mar 12 2021, 1:52 PM
SeanP created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMar 12 2021, 1:52 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
clang/include/clang/Driver/Driver.h
160

I'm seeing code left unchanged like:

TheDriver.CCPrintOptionsFilename = ::getenv("CC_PRINT_OPTIONS_FILE");

Assigning to a std::string from a null char * is known to cause things like

Segmentation fault
clang/lib/Driver/Compilation.cpp
173

Please update for the clang-format suggestions.

SeanP updated this revision to Diff 332836.Mar 23 2021, 5:44 PM

Changed the code checking the env vars so it doesn't assign a null_ptr to the string.

SeanP marked 2 inline comments as done.Mar 23 2021, 5:46 PM
SeanP added inline comments.
clang/include/clang/Driver/Driver.h
160

I've fixed in newest patch.

LGTM; thanks.

clang/lib/Driver/Driver.cpp
4043

Just noting that this means having the environment variable set (but empty) will now "work" instead of generating an error.

This revision is now accepted and ready to land.Mar 24 2021, 3:05 PM
This revision was automatically updated to reflect the committed changes.