diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fb90068c..1fbccae5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -18,7 +18,7 @@ jobs: - uses: actions/checkout@v4 - name: install dependencies - run: sudo apt-get update && sudo apt-get install -y llvm-14 clang-14 libc++-14-dev libc++abi-14-dev python3-minimal libgoogle-perftools-dev libboost-container-dev python3-dev libbsd-dev + run: sudo apt-get update && sudo apt-get install -y llvm-18 clang-18 libc++-18-dev libc++abi-18-dev libunwind-18-dev python3-minimal libgoogle-perftools-dev libboost-container-dev python3-dev libbsd-dev - name: Cache Z3 id: cache-z3 @@ -51,13 +51,13 @@ jobs: path: ${{ github.workspace }}/aflpp - name: configure - run: CC=clang-14 CXX=clang++-14 cmake -B ${{ github.workspace }}/build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=${{ github.workspace }}/install -DAFLPP_PATH=${{ github.workspace }}/aflpp + run: CC=clang-18 CXX=clang++-18 cmake -B ${{ github.workspace }}/build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=${{ github.workspace }}/install -DAFLPP_PATH=${{ github.workspace }}/aflpp -DLLVM_DIR=/usr/lib/llvm-18/lib/cmake/llvm - name: build - run: CC=clang-14 CXX=clang++-14 cmake --build ${{ github.workspace }}/build + run: CC=clang-18 CXX=clang++-18 cmake --build ${{ github.workspace }}/build - name: install - run: CC=clang-14 CXX=clang++-14 cmake --install ${{ github.workspace }}/build + run: CC=clang-18 CXX=clang++-18 cmake --install ${{ github.workspace }}/build - name: install lit run: pip install lit diff --git a/CMakeLists.txt b/CMakeLists.txt index 718d83c6..0a43ef50 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.13) project(symsan VERSION 1.2.2 LANGUAGES C CXX ASM) -find_package(LLVM 14 REQUIRED CONFIG) +find_package(LLVM REQUIRED CONFIG) # Find Z3 (minimum version 4.8.15 required for string theory APIs) # Prefer /usr/local over system @@ -44,6 +44,13 @@ if (LLVM_FOUND) message(STATUS "LLVM_VERSION_MAJOR: ${LLVM_VERSION_MAJOR}") message(STATUS "LLVM_VERSION_MINOR: ${LLVM_VERSION_MINOR}") message(STATUS "LLVM_VERSION_PATCH: ${LLVM_VERSION_PATCH}") + # SymSan requires LLVM >= 18 (the default on Ubuntu 24.04). Older versions + # predate opaque pointers and the DFSan trampoline removal and are no longer + # supported. (find_package(LLVM 18) can't be used: LLVM's version config + # requires an exact major.minor match.) + if (LLVM_VERSION_MAJOR LESS 18) + message(FATAL_ERROR "SymSan requires LLVM >= 18, found ${LLVM_VERSION_MAJOR}.${LLVM_VERSION_MINOR}") + endif() else() message(FATAL_ERROR "You haven't install LLVM !") endif() diff --git a/README.md b/README.md index 596c3aa3..ac0ae663 100644 --- a/README.md +++ b/README.md @@ -23,14 +23,14 @@ it has more strict dependency on the LLVM version. Right now only LLVM 12 is tes ### Build Requirements - Linux-amd64 (Tested on Ubuntu 24.04) -- [LLVM 14.0.6](http://llvm.org/docs/index.html): clang, libc++, libc++abi +- [LLVM 18.1.18](http://llvm.org/docs/index.html): clang, libc++, libc++abi ### Compilation Create a `build` directory and execute the following commands in it: ```shell -$ CC=clang-14 CXX=clang++-14 cmake -DCMAKE_INSTALL_PREFIX=/path/to/install -DCMAKE_BUILD_TYPE=Release /path/to/symsan/source +$ CC=clang-18 CXX=clang++-18 cmake -DCMAKE_INSTALL_PREFIX=/path/to/install -DCMAKE_BUILD_TYPE=Release /path/to/symsan/source $ make $ make install ``` diff --git a/compiler/CMakeLists.txt b/compiler/CMakeLists.txt index adb282f2..6df05128 100644 --- a/compiler/CMakeLists.txt +++ b/compiler/CMakeLists.txt @@ -1,5 +1,9 @@ add_executable(KOClang ko_clang.c) set_target_properties(KOClang PROPERTIES OUTPUT_NAME "ko-clang") +# Let the driver version-gate the clang flags it forwards. +target_compile_definitions(KOClang PRIVATE + LLVM_VERSION_MAJOR=${LLVM_VERSION_MAJOR} + LLVM_VERSION_MINOR=${LLVM_VERSION_MINOR}) add_custom_command(TARGET KOClang POST_BUILD COMMAND ln -sf "ko-clang" "ko-clang++") diff --git a/compiler/ko_clang.c b/compiler/ko_clang.c index 0efbae0c..47aa6fc8 100644 --- a/compiler/ko_clang.c +++ b/compiler/ko_clang.c @@ -444,7 +444,11 @@ static void edit_params(u32 argc, char **argv) { } if (!skip_instrumentation) { + // The new pass manager is the default since clang 13 and the flag that + // used to request it was removed in clang 16. +#if LLVM_VERSION_CODE < LLVM_VERSION(16, 0) cc_params[cc_par_cnt++] = "-fexperimental-new-pass-manager"; +#endif // add UCSanPass first, if specified if (use_ucsan) { add_ucsan_pass(); diff --git a/compiler/ucsan_opt b/compiler/ucsan_opt index 261e4b7d..9a7dd9f7 100755 --- a/compiler/ucsan_opt +++ b/compiler/ucsan_opt @@ -13,15 +13,13 @@ else: print(f"Cannot find {ucsan_pass_path}, please make sure you have run this script in the right directory(the directory ko-clang installed)") os._exit(1) -KO_FLAGS = ["KO_CC=clang-14"] +KO_FLAGS = ["KO_CC=clang-18"] ko_flags = ' '.join(KO_FLAGS) -# Base opt-14 command: load UCSan pass -# Load library twice for LLVM-14: -# -load: registers cl::opt definitions (command-line options) -# -load-pass-plugin: registers the new pass manager plugin -cc = f"opt-14 -load {path_to_install}/lib/symsan/UCSanPass.so" \ - f" -load-pass-plugin={path_to_install}/lib/symsan/UCSanPass.so" \ +# Base opt command: load the UCSan pass plugin. On LLVM 18 -load-pass-plugin +# alone registers both the new-PM pipeline and the pass's cl::opt options (the +# separate -load needed on LLVM 14 is no longer required). +cc = f"opt-18 -load-pass-plugin={path_to_install}/lib/symsan/UCSanPass.so" \ f" -ucsan-abilist={path_to_install}/lib/symsan/ucsan_abilist.txt" ko_cc = f"{path_to_install}/bin/ko-clang" @@ -66,8 +64,8 @@ if not files: argparser.print_usage() os._exit(1) -# Note: With LLVM-14 new pass manager, we don't add -O# flags to opt -# Optimization is controlled via KO_DONT_OPTIMIZE and handled by ko-clang +# Note: with the new pass manager we don't add -O# flags to opt; +# optimization is controlled via KO_DONT_OPTIMIZE and handled by ko-clang if not args.optimization: KO_FLAGS.append("KO_DONT_OPTIMIZE=1") @@ -98,9 +96,8 @@ if args.taint: os._exit(1) # Tell UCSanPass that TaintPass will run after it cc += f" -ucsan-with-taint=true" - # Load TaintPass library for cl::opt and pass registration - cc += f" -load {taint_pass_path}" \ - f" -load-pass-plugin={taint_pass_path}" \ + # Load TaintPass plugin (registers its pipeline and cl::opt options) + cc += f" -load-pass-plugin={taint_pass_path}" \ f" -taint-abilist={path_to_install}/lib/symsan/dfsan_abilist.txt" \ f" -taint-with-ucsan=true" \ f" -taint-solve-ub=true" @@ -138,7 +135,7 @@ for file in files: cleanup() os._exit(1) tmp_files.append(bc_file) - ret = command(f"llc-14 -filetype=obj --relocation-model=pic -o {obj_file} {bc_file}") + ret = command(f"llc-18 -filetype=obj --relocation-model=pic -o {obj_file} {bc_file}") if ret != 0: print(f"Failed to compile {file}, check the error message above") cleanup() diff --git a/include/version.h b/include/version.h index 9407b3a3..8081a95a 100644 --- a/include/version.h +++ b/include/version.h @@ -5,43 +5,14 @@ #define LLVM_VERSION(major, minor) ((major)*100 + (minor)) #define LLVM_VERSION_CODE LLVM_VERSION(LLVM_VERSION_MAJOR, LLVM_VERSION_MINOR) -#if LLVM_VERSION_CODE >= LLVM_VERSION(5, 0) -#define LLVM_ATTRIBUTE_LIST AttributeList - -#define LLVM_NEW_ALLOCINST(ty, name, insertp) \ - (new AllocaInst(ty, getDataLayout().getAllocaAddrSpace(), name, insertp)) - -#define LLVM_REMOVE_ATTRIBUTE(func, attr, attrbuilder) \ - func->removeAttributes(attr, attrbuilder) - -#else - -#define LLVM_ATTRIBUTE_LIST AttributeSet - -#define LLVM_NEW_ALLOCINST(ty, name, insertp) \ - (new AllocaInst(ty, name, insertp)) - -#define LLVM_REMOVE_ATTRIBUTE(func, attr, attrbuilder) \ - func->removeAttributes( \ - attr, LLVM_ATTRIBUTE_LIST::get(func->getContext(), attr, attrbuilder)) - -#endif - -#if LLVM_VERSION_CODE >= LLVM_VERSION(6, 0) - -#define SCL_INSECTION(scl, section, prefix, query, category) \ - scl->inSection(section, prefix, query, category) - -#define LLVM_ADD_PARAM_ATTR(func, argno, attr) func->addParamAttr(argno, attr) - -#else - -#define SCL_INSECTION(scl, section, prefix, query, category) \ - scl->inSection(prefix, query, category) - -#define LLVM_ADD_PARAM_ATTR(func, argno, attr) \ - func->addAttribute(argno + 1, attr) - +// SymSan targets LLVM 18 (the default on Ubuntu 24.04) as its minimum. These +// headers are only needed by C++ translation units built against LLVM; the +// compiler-driver C sources include this file only for the LLVM_VERSION macros +// above. +#if defined(__cplusplus) && defined(LLVM_VERSION_MAJOR) +#include +#include "llvm/TargetParser/Triple.h" +#include "llvm/IR/AttributeMask.h" #endif #endif diff --git a/instrumentation/CMakeLists.txt b/instrumentation/CMakeLists.txt index 2afdd0a5..e71183ec 100644 --- a/instrumentation/CMakeLists.txt +++ b/instrumentation/CMakeLists.txt @@ -1,4 +1,4 @@ -set (CMAKE_CXX_STANDARD 14) +set (CMAKE_CXX_STANDARD 17) set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -g") # fix pass bug: https://github.com/sampsyo/llvm-pass-skeleton/issues/7#issuecomment-401834287 set (CMAKE_MODULE_LINKER_FLAGS "${CMAKE_CXX_LINK_FLAGS} -Wl,-znodelete") diff --git a/instrumentation/LoopOutlinePass.cpp b/instrumentation/LoopOutlinePass.cpp index cb47bb98..84ffe632 100644 --- a/instrumentation/LoopOutlinePass.cpp +++ b/instrumentation/LoopOutlinePass.cpp @@ -26,6 +26,7 @@ //===----------------------------------------------------------------------===// #include "llvm/ADT/SetVector.h" +#include "llvm/BinaryFormat/Dwarf.h" #include "llvm/ADT/SmallVector.h" #include "llvm/Analysis/LoopInfo.h" #include "llvm/IR/BasicBlock.h" @@ -207,7 +208,7 @@ collectGlobals(Function *Out) { for (Instruction &I : BB) for (Value *Op : I.operands()) if (auto *gv = dyn_cast(Op->stripPointerCasts())) - if (!gv->getName().startswith("llvm.") && !gv->getName().empty()) + if (!(gv->getName()).starts_with("llvm.") && !gv->getName().empty()) gvs.insert(gv); std::vector> out; for (GlobalVariable *gv : gvs) { @@ -240,10 +241,12 @@ static Function *extractTarget(const Target &T, unsigned Idx) { DT.recalculate(*T.F); CodeExtractorAnalysisCache CEAC(*T.F); + // NOTE: the trailing Suffix argument is intentionally left at its default; + // LLVM 18 inserted an AllocationBlock parameter before it, so passing it + // positionally is not source-compatible across versions. CodeExtractor CE(T.Blocks, &DT, /*AggregateArgs=*/false, /*BFI=*/nullptr, /*BPI=*/nullptr, /*AC=*/nullptr, - /*AllowVarArgs=*/false, /*AllowAlloca=*/false, - /*Suffix=*/""); + /*AllowVarArgs=*/false, /*AllowAlloca=*/false); if (!CE.isEligible()) { if (ClVerbose) errs() << "[loop-outline] skip " << T.F->getName() @@ -274,7 +277,7 @@ struct LoopOutlinePass : public PassInfoMixin { // Phase 1 (read-only): gather innermost-loop body regions. std::vector Targets; for (Function &F : M) { - if (F.isDeclaration() || F.getName().startswith("__ucsan_loopbody_")) + if (F.isDeclaration() || (F.getName()).starts_with("__ucsan_loopbody_")) continue; DominatorTree DT; DT.recalculate(F); diff --git a/instrumentation/TaintPass.cpp b/instrumentation/TaintPass.cpp index 2ae406dc..615928dc 100644 --- a/instrumentation/TaintPass.cpp +++ b/instrumentation/TaintPass.cpp @@ -14,17 +14,15 @@ //#include "defs.h" #include "UCSanSummary.h" -#include "version.h" +#include #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/DepthFirstIterator.h" -#include "llvm/ADT/None.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" -#include "llvm/ADT/Triple.h" #include "llvm/ADT/iterator.h" #include "llvm/Analysis/AssumptionCache.h" #include "llvm/Analysis/LoopInfo.h" @@ -34,6 +32,7 @@ #include "llvm/Analysis/ValueTracking.h" #include "llvm/Transforms/Utils/ScalarEvolutionExpander.h" #include "llvm/IR/Argument.h" +#include "llvm/IR/AttributeMask.h" #include "llvm/IR/Attributes.h" #include "llvm/IR/BasicBlock.h" #include "llvm/IR/CFG.h" @@ -48,6 +47,7 @@ #include "llvm/IR/GlobalVariable.h" #include "llvm/IR/IRBuilder.h" #include "llvm/IR/InlineAsm.h" +#include "llvm/IR/InstIterator.h" #include "llvm/IR/InstVisitor.h" #include "llvm/IR/InstrTypes.h" #include "llvm/IR/Instruction.h" @@ -73,8 +73,8 @@ #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/SpecialCaseList.h" #include "llvm/Support/VirtualFileSystem.h" +#include "llvm/TargetParser/Triple.h" #include "llvm/Transforms/Instrumentation.h" -#include "llvm/Transforms/IPO/PassManagerBuilder.h" #include "llvm/Transforms/Utils/BasicBlockUtils.h" #include "llvm/Transforms/Utils/Local.h" #include @@ -206,6 +206,17 @@ static cl::opt ClWithUCSan( cl::desc("Performs under-constrained symbolic execution."), cl::Hidden, cl::init(false)); +// SYMSAN specific flag. Upstream DFSan conservatively stores a zero shadow on +// atomic stores (and CAS/RMW) to avoid shadow data races. That loses the +// symbolic expression of the stored value. Concolic targets are typically +// single-threaded, so by default we preserve the real shadow instead. Set to +// false to restore DFSan's race-free zeroing behaviour. +static cl::opt ClPreserveAtomicShadow( + "taint-preserve-atomic-shadow", + cl::desc("Propagate the real shadow through atomic stores/CAS instead of " + "zeroing it (loses symex but is race-free when disabled)."), + cl::Hidden, cl::init(true)); + static StringRef getGlobalTypeString(const GlobalValue &G) { // Types of GlobalVariables are always pointer types. Type *GType = G.getValueType(); @@ -339,7 +350,7 @@ TransformFunctionAttributes(const TransformedFunction& TransformedFunction, return AttributeList::get(Ctx, CallSiteAttrs.getFnAttrs(), CallSiteAttrs.getRetAttrs(), - llvm::makeArrayRef(ArgumentAttributes)); + ArrayRef(ArgumentAttributes)); } class Taint { @@ -409,6 +420,7 @@ class Taint { FunctionType *TaintUnionStoreFnTy; FunctionType *TaintGEPOffsetFnTy; FunctionType *TaintUnimplementedFnTy; + FunctionType *TaintWrapperExternWeakNullFnTy; FunctionType *TaintSetLabelFnTy; FunctionType *TaintNonzeroLabelFnTy; FunctionType *TaintVarargWrapperFnTy; @@ -434,6 +446,7 @@ class Taint { FunctionCallee TaintUnionStoreFn; FunctionCallee TaintGEPOffsetFn; FunctionCallee TaintUnimplementedFn; + FunctionCallee TaintWrapperExternWeakNullFn; FunctionCallee TaintSetLabelFn; FunctionCallee TaintNonzeroLabelFn; FunctionCallee TaintVarargWrapperFn; @@ -471,14 +484,13 @@ class Taint { bool isInstrumented(const GlobalAlias *GA); FunctionType *getArgsFunctionType(FunctionType *T); bool isForceZeroLabels(const Function *F); - FunctionType *getTrampolineFunctionType(FunctionType *T); TransformedFunction getCustomFunctionType(FunctionType *T); WrapperKind getWrapperKind(Function *F); void addGlobalNameSuffix(GlobalValue *GV); + void buildExternWeakCheckIfNeeded(IRBuilder<> &IRB, Function *F); Function *buildWrapperFunction(Function *F, StringRef NewFName, GlobalValue::LinkageTypes NewFLink, FunctionType *NewFT); - Constant *getOrBuildTrampolineFunction(FunctionType *FT, StringRef FName); void addContextRecording(Function &F); void addFrameTracing(Function &F); @@ -676,7 +688,7 @@ class TaintVisitor : public InstVisitor { void visitLoadInst(LoadInst &LI); void visitStoreInst(StoreInst &SI); void visitAtomicRMWInst(AtomicRMWInst &I); - //void visitAtomicCmpXchgInst(AtomicCmpXchgInst &I); + void visitAtomicCmpXchgInst(AtomicCmpXchgInst &I); void visitReturnInst(ReturnInst &RI); void visitCallBase(CallBase &CB); void visitPHINode(PHINode &PN); @@ -732,24 +744,6 @@ FunctionType *Taint::getArgsFunctionType(FunctionType *T) { return FunctionType::get(RetType, ArgTypes, T->isVarArg()); } -FunctionType *Taint::getTrampolineFunctionType(FunctionType *T) { - assert(!T->isVarArg()); - SmallVector ArgTypes; - ArgTypes.push_back(T->getPointerTo()); - ArgTypes.append(T->param_begin(), T->param_end()); - // we keep the shadow type consistent with the arg type so we don't - // need to collapse or expand the shadow - for (unsigned i = 0, ie = T->getNumParams(); i != ie; ++i) { - Type* param_type = T->getParamType(i); - ArgTypes.push_back(getShadowTy(param_type)); - } - // ArgTypes.append(T->getNumParams(), PrimitiveShadowTy); - Type *RetType = T->getReturnType(); - if (!RetType->isVoidTy()) - // ArgTypes.push_back(PrimitiveShadowPtrTy); - ArgTypes.push_back(PointerType::getUnqual(getShadowTy(RetType))); - return FunctionType::get(T->getReturnType(), ArgTypes, false); -} TransformedFunction Taint::getCustomFunctionType(FunctionType *T) { SmallVector ArgTypes; @@ -761,16 +755,10 @@ TransformedFunction Taint::getCustomFunctionType(FunctionType *T) { std::vector ArgumentIndexMapping; for (unsigned I = 0, E = T->getNumParams(); I != E; ++I) { Type* ParamType = T->getParamType(I); - FunctionType *FT; - if (isa(ParamType) && - (FT = dyn_cast(ParamType->getPointerElementType()))) { - ArgumentIndexMapping.push_back(ArgTypes.size()); - ArgTypes.push_back(getTrampolineFunctionType(FT)->getPointerTo()); - ArgTypes.push_back(Type::getInt8PtrTy(*Ctx)); - } else { - ArgumentIndexMapping.push_back(ArgTypes.size()); - ArgTypes.push_back(ParamType); - } + // Opaque pointers hide the pointee type, so custom-wrapper trampolines for + // function-pointer arguments cannot be detected; pass the parameter through. + ArgumentIndexMapping.push_back(ArgTypes.size()); + ArgTypes.push_back(ParamType); } for (unsigned i = 0, e = T->getNumParams(); i != e; ++i) { // we keep the shadow type consistent with the arg type so we don't @@ -898,7 +886,7 @@ void Taint::addContextRecording(Function &F) { // Strip dfs$ prefix auto FName = F.getName(); - if (FName.startswith("dfs")) { + if ((FName).starts_with("dfs")) { size_t pos = FName.find_first_of('$'); FName = FName.drop_front(pos + 1); } @@ -910,10 +898,10 @@ void Taint::addContextRecording(Function &F) { ConstantInt *CID = ConstantInt::get(Int32Ty, hash); LoadInst *LCS = IRB.CreateLoad(Int32Ty, CallStack); - LCS->setMetadata(Mod->getMDKindID("nosanitize"), MDNode::get(*Ctx, None)); + LCS->setMetadata(Mod->getMDKindID("nosanitize"), MDNode::get(*Ctx, std::nullopt)); Value *NCS = IRB.CreateXor(LCS, CID); StoreInst *SCS = IRB.CreateStore(NCS, CallStack); - SCS->setMetadata(Mod->getMDKindID("nosanitize"), MDNode::get(*Ctx, None)); + SCS->setMetadata(Mod->getMDKindID("nosanitize"), MDNode::get(*Ctx, std::nullopt)); // Recover ctx at the end of a function for (auto FI = F.begin(), FE = F.end(); FI != FE; FI++) { @@ -922,7 +910,7 @@ void Taint::addContextRecording(Function &F) { if (isa(Inst) || isa(Inst)) { IRB.SetInsertPoint(Inst); SCS = IRB.CreateStore(LCS, CallStack); - SCS->setMetadata(Mod->getMDKindID("nosanitize"), MDNode::get(*Ctx, None)); + SCS->setMetadata(Mod->getMDKindID("nosanitize"), MDNode::get(*Ctx, std::nullopt)); } } } @@ -997,15 +985,19 @@ bool Taint::initializeModule(Module &M) { PrimitiveShadowTy, { PrimitiveShadowTy, VoidPtrTy, VoidPtrTy }, /*isVarArg=*/ false); TaintUnimplementedFnTy = FunctionType::get( - Type::getVoidTy(*Ctx), Type::getInt8PtrTy(*Ctx), /*isVarArg=*/false); - Type *TaintSetLabelArgs[3] = { PrimitiveShadowTy, Type::getInt8PtrTy(*Ctx), + Type::getVoidTy(*Ctx), PointerType::getUnqual(*Ctx), /*isVarArg=*/false); + Type *TaintWrapperExternWeakNullArgs[2] = { PointerType::getUnqual(*Ctx), + PointerType::getUnqual(*Ctx) }; + TaintWrapperExternWeakNullFnTy = FunctionType::get( + Type::getVoidTy(*Ctx), TaintWrapperExternWeakNullArgs, /*isVarArg=*/false); + Type *TaintSetLabelArgs[3] = { PrimitiveShadowTy, PointerType::getUnqual(*Ctx), IntptrTy }; TaintSetLabelFnTy = FunctionType::get(Type::getVoidTy(*Ctx), TaintSetLabelArgs, /*isVarArg=*/false); TaintNonzeroLabelFnTy = FunctionType::get( - Type::getVoidTy(*Ctx), None, /*isVarArg=*/false); + Type::getVoidTy(*Ctx), std::nullopt, /*isVarArg=*/false); TaintVarargWrapperFnTy = FunctionType::get( - Type::getVoidTy(*Ctx), Type::getInt8PtrTy(*Ctx), /*isVarArg=*/false); + Type::getVoidTy(*Ctx), PointerType::getUnqual(*Ctx), /*isVarArg=*/false); Type *TaintTraceCmpArgs[7] = { PrimitiveShadowTy, PrimitiveShadowTy, Int32Ty, Int32Ty, Int64Ty, Int64Ty, Int32Ty }; TaintTraceCmpFnTy = FunctionType::get( @@ -1047,7 +1039,7 @@ bool Taint::initializeModule(Module &M) { // __taint_solve_str_bounds(str_ptr, buf_label, buf_ptr, step) TaintSolveStrBoundsFnTy = FunctionType::get( Type::getVoidTy(*Ctx), - { Type::getInt8PtrTy(*Ctx), PrimitiveShadowTy, Int64Ty, Int64Ty }, false); + { PointerType::getUnqual(*Ctx), PrimitiveShadowTy, Int64Ty, Int64Ty }, false); TaintTraceGlobalFnTy = FunctionType::get( PrimitiveShadowTy, { Int64Ty, Int64Ty }, false); @@ -1130,6 +1122,22 @@ void Taint::addGlobalNameSuffix(GlobalValue *GV) { } } +void Taint::buildExternWeakCheckIfNeeded(IRBuilder<> &IRB, Function *F) { + // If the function we are wrapping was ExternWeak, it may be null. + // The original code before calling this wrapper may have checked for null, + // but replacing with a known-to-not-be-null wrapper can break this check. + // When replacing uses of the extern weak function with the wrapper we try + // to avoid replacing uses in conditionals, but this is not perfect. + // In the case where we fail, and accidentially optimize out a null check + // for a extern weak function, add a check here to help identify the issue. + if (GlobalValue::isExternalWeakLinkage(F->getLinkage())) { + std::vector Args; + Args.push_back(IRB.CreatePointerCast(F, PointerType::getUnqual(*Ctx))); + Args.push_back(IRB.CreateGlobalStringPtr(F->getName())); + IRB.CreateCall(TaintWrapperExternWeakNullFn, Args); + } +} + Function * Taint::buildWrapperFunction(Function *F, StringRef NewFName, GlobalValue::LinkageTypes NewFLink, @@ -1163,45 +1171,6 @@ Taint::buildWrapperFunction(Function *F, StringRef NewFName, return NewF; } -Constant *Taint::getOrBuildTrampolineFunction(FunctionType *FT, - StringRef FName) { - FunctionType *FTT = getTrampolineFunctionType(FT); - FunctionCallee C = Mod->getOrInsertFunction(FName, FTT); - Function *F = dyn_cast(C.getCallee()); - if (F && F->isDeclaration()) { - F->setLinkage(GlobalValue::LinkOnceODRLinkage); - BasicBlock *BB = BasicBlock::Create(*Ctx, "entry", F); - std::vector Args; - Function::arg_iterator AI = F->arg_begin() + 1; - for (unsigned N = FT->getNumParams(); N != 0; ++AI, --N) - Args.push_back(&*AI); - CallInst *CI = CallInst::Create(FT, &*F->arg_begin(), Args, "", BB); - Type *RetType = FT->getReturnType(); - ReturnInst *RI = RetType->isVoidTy() ? ReturnInst::Create(*Ctx, BB) - : ReturnInst::Create(*Ctx, CI, BB); - - // F is called by a wrapped custom function with primitive shadows. So - // its arguments and return value need conversion. - TaintFunction TF(*this, F, /*IsNativeABI=*/true, - /*IsForceZeroLabels=*/false); - Function::arg_iterator ValAI = F->arg_begin(), ShadowAI = AI; - ++ValAI; - for (unsigned N = FT->getNumParams(); N != 0; ++ValAI, ++ShadowAI, --N) { - // we don't collapse or expand the shadow - TF.ValShadowMap[&*ValAI] = &*ShadowAI; - } - Function::arg_iterator RetShadowAI = ShadowAI; - TaintVisitor(TF).visitCallInst(*CI); - if (!RetType->isVoidTy()) { - // we don't collapse or expand the shadow - new StoreInst(TF.getShadow(RI->getReturnValue()), - &*std::prev(F->arg_end()), RI); - } - } - - return cast(C.getCallee()); -} - // Initialize DataFlowSanitizer runtime functions and declare them in the module void Taint::initializeRuntimeFunctions(Module &M) { { @@ -1239,6 +1208,10 @@ void Taint::initializeRuntimeFunctions(Module &M) { TaintUnimplementedFn = Mod->getOrInsertFunction("__dfsan_unimplemented", TaintUnimplementedFnTy); } + { + TaintWrapperExternWeakNullFn = Mod->getOrInsertFunction( + "__dfsan_wrapper_extern_weak_null", TaintWrapperExternWeakNullFnTy); + } { AttributeList AL; AL = AL.addParamAttribute(M.getContext(), 0, Attribute::ZExt); @@ -1275,6 +1248,8 @@ void Taint::initializeRuntimeFunctions(Module &M) { TaintGEPOffsetFn.getCallee()->stripPointerCasts()); TaintRuntimeFunctions.insert( TaintUnimplementedFn.getCallee()->stripPointerCasts()); + TaintRuntimeFunctions.insert( + TaintWrapperExternWeakNullFn.getCallee()->stripPointerCasts()); TaintRuntimeFunctions.insert( TaintSetLabelFn.getCallee()->stripPointerCasts()); TaintRuntimeFunctions.insert( @@ -1594,7 +1569,40 @@ bool Taint::runImpl(Module &M) { Value *WrappedFnCst = ConstantExpr::getBitCast(NewF, PointerType::getUnqual(FT)); - F.replaceAllUsesWith(WrappedFnCst); + + // Extern weak functions can sometimes be null at execution time. + // Code will sometimes check if an extern weak function is null. + // This could look something like: + // declare extern_weak i8 @my_func(i8) + // br i1 icmp ne (i8 (i8)* @my_func, i8 (i8)* null), label %use_my_func, + // label %avoid_my_func + // The @"dfsw$my_func" wrapper is never null, so if we replace this use + // in the comparision, the icmp will simplify to false and we have + // accidentially optimized away a null check that is necessary. + // This can lead to a crash when the null extern_weak my_func is called. + // + // To prevent (the most common pattern of) this problem, + // do not replace uses in comparisons with the wrapper. + // We definitely want to replace uses in call instructions. + // Other uses (e.g. store the function address somewhere) might be + // called or compared or both - this case may not be handled correctly. + // We will default to replacing with wrapper in cases we are unsure. + auto IsNotCmpUse = [](Use &U) -> bool { + User *Usr = U.getUser(); + if (ConstantExpr *CE = dyn_cast(Usr)) { + // This is the most common case for icmp ne null + if (CE->getOpcode() == Instruction::ICmp) { + return false; + } + } + if (Instruction *I = dyn_cast(Usr)) { + if (I->getOpcode() == Instruction::ICmp) { + return false; + } + } + return true; + }; + F.replaceUsesWithIf(WrappedFnCst, IsNotCmpUse); UnwrappedFnMap[WrappedFnCst] = &F; *FI = NewF; @@ -1628,7 +1636,7 @@ bool Taint::runImpl(Module &M) { continue; addContextRecording(*F); - if (!F->getName().startswith("dfsw$")) + if (!(F->getName()).starts_with("dfsw$")) addFrameTracing(*F); removeUnreachableBlocks(*F); @@ -1807,7 +1815,7 @@ bool TaintFunction::handleUCSanCall(CallInst *CI, Instruction *Next) { if (!Callee) return false; StringRef FName = Callee->getName(); - if (!FName.startswith("__dfsw_")) + if (!(FName).starts_with("__dfsw_")) return false; StringRef BaseName = FName.drop_front(7); // skip "__dfsw_" @@ -2195,7 +2203,7 @@ void TaintFunction::hoistBoundsChecks() { continue; IRBuilder<> IRB(CI); - Type *I8PtrTy = Type::getInt8PtrTy(F->getContext()); + Type *I8PtrTy = PointerType::getUnqual(F->getContext()); Type *I8PtrPtrTy = PointerType::getUnqual(I8PtrTy); Value *ArgI8 = IRB.CreateBitCast(Arg, I8PtrTy); Value *FieldAddr = ArgI8; @@ -2334,7 +2342,7 @@ void TaintFunction::hoistBoundsChecks() { // __taint_solve_str_bounds(str_ptr, buf_label, buf_ptr, step) Instruction *InsertPt = Preheader->getTerminator(); IRBuilder<> IRB(InsertPt); - Value *StrPtr = IRB.CreateBitCast(StrBase, Type::getInt8PtrTy(*TT.Ctx)); + Value *StrPtr = IRB.CreateBitCast(StrBase, PointerType::getUnqual(*TT.Ctx)); if (StrUCChk) { // Hoist ucsan_check_pointer with deref=1 so the string object // is materialized before __taint_solve_str_bounds dereferences it @@ -2686,7 +2694,7 @@ void TaintFunction::hoistBoundsChecks() { TripCount, SE.getConstant(TripCount->getType(), Summary.AccessSize)); IRBuilder<> IRB(InsertPt); - Type *I8PtrTy = Type::getInt8PtrTy(F->getContext()); + Type *I8PtrTy = PointerType::getUnqual(F->getContext()); Type *I8PtrPtrTy = PointerType::getUnqual(I8PtrTy); Value *ArgI8 = IRB.CreateBitCast(Arg, I8PtrTy); Value *FieldAddr = ArgI8; @@ -3018,7 +3026,7 @@ Value *TaintFunction::loadShadowRecursive( uint64_t SubSize = DL.getTypeStoreSize(SubTy); assert(Size >= SubSize); uint64_t SubSizeInBits = DL.getTypeSizeInBits(SubTy); - Align = std::min(Align, (uint64_t)DL.getABITypeAlignment(SubTy)); + Align = std::min(Align, (uint64_t)(DL).getABITypeAlign(SubTy).value()); // load a primitive shadow from address Value *PrimitiveShadow = loadPrimitiveShadow(Addr, SubSize, SubSizeInBits, Align, IRB); // then insert the primitive shadow into the sub-field @@ -3212,6 +3220,59 @@ void TaintVisitor::visitAtomicRMWInst(AtomicRMWInst &I) { I.setOrdering(addReleaseOrdering(I.getOrdering())); } +void TaintVisitor::visitAtomicCmpXchgInst(AtomicCmpXchgInst &I) { + auto &DL = I.getModule()->getDataLayout(); + Value *Ptr = I.getPointerOperand(); + Value *NewVal = I.getNewValOperand(); + Type *ValTy = NewVal->getType(); + uint64_t Size = DL.getTypeStoreSize(ValTy); + + // The result is { ValTy old_value, i1 success }. Field 0 carries the value + // read from memory; the success flag is a concrete comparison result and is + // treated as clean. + Type *ResShadowTy = TF.TT.getShadowTy(I.getType()); + + if (Size == 0) { + TF.setShadow(&I, TF.TT.getZeroShadow(&I)); + // Upstream DFSan follows MSan's ordering change; do the same. + I.setSuccessOrdering(addReleaseOrdering(I.getSuccessOrdering())); + return; + } + + if (!ClPreserveAtomicShadow) { + // Conservative, race-free behaviour matching upstream DFSan: zero the + // shadow at the stored address and return a zero result shadow. + Value *Zero = TF.TT.getZeroShadow(ValTy); + TF.storeShadow(Ptr, ValTy, Size, I.getAlign(), Zero, &I); + TF.setShadow(&I, TF.TT.getZeroShadow(&I)); + I.setSuccessOrdering(addReleaseOrdering(I.getSuccessOrdering())); + return; + } + + // Shadow of the value currently in memory (returned in field 0). + Value *OldShadow = TF.loadShadow(ValTy, Ptr, Size, I.getAlign(), &I); + Value *NewShadow = TF.getShadow(NewVal); + + // The exchange only writes NewVal on success, so the shadow in memory + // becomes NewShadow when the comparison succeeded and stays OldShadow + // otherwise. The success flag is only available after the instruction. + Instruction *Pos = I.getNextNode(); + IRBuilder<> IRB(Pos); + Value *Success = IRB.CreateExtractValue(&I, 1); + Value *StoredShadow = IRB.CreateSelect(Success, NewShadow, OldShadow); + TF.storeShadow(Ptr, ValTy, Size, I.getAlign(), StoredShadow, Pos); + + Value *ResShadow = UndefValue::get(ResShadowTy); + ResShadow = IRB.CreateInsertValue(ResShadow, OldShadow, 0); + ResShadow = IRB.CreateInsertValue( + ResShadow, TF.TT.getZeroShadow(I.getType()->getStructElementType(1)), 1); + TF.setShadow(&I, ResShadow); + + // TODO: The ordering change follows MSan. It is possible not to change + // ordering because we always set and use 0 shadows. + I.setSuccessOrdering(addReleaseOrdering(I.getSuccessOrdering())); +} + void TaintVisitor::visitLoadInst(LoadInst &LI) { auto &DL = LI.getModule()->getDataLayout(); uint64_t Size = DL.getTypeStoreSize(LI.getType()); @@ -3259,7 +3320,7 @@ void TaintFunction::storeShadowRecursive( if (!isa(SubShadowTy) && !isa(SubShadowTy)) { uint64_t SubSize = DL.getTypeStoreSize(SubShadowTy); assert(Size >= SubSize); - Align = std::min(Align, (uint64_t)DL.getABITypeAlignment(SubShadowTy)); + Align = std::min(Align, (uint64_t)(DL).getABITypeAlign(SubShadowTy).value()); // load a primitive shadow from the sub-field Value *PrimitiveShadow = IRB.CreateExtractValue(Shadow, Indices); // then store the primitive shadow into the shadow address @@ -3366,7 +3427,9 @@ void TaintVisitor::visitStoreInst(StoreInst &SI) { if (SI.isAtomic()) SI.setOrdering(addReleaseOrdering(SI.getOrdering())); - Value* Shadow = SI.isAtomic() ? TF.TT.getZeroShadow(VT) : TF.getShadow(Val); + Value* Shadow = (SI.isAtomic() && !ClPreserveAtomicShadow) + ? TF.TT.getZeroShadow(VT) + : TF.getShadow(Val); // check bounds first if (ClTraceBound) @@ -3688,12 +3751,12 @@ void TaintVisitor::visitAllocaInst(AllocaInst &I) { IRBuilder<> IRB(I.getNextNode()); auto DL = I.getModule()->getDataLayout(); auto size = I.getAllocationSizeInBits(DL); - assert(size != None); + assert(size != std::nullopt); Value *Size = ConstantInt::get(TF.TT.IntptrTy, (size->getFixedValue() + 7) >> 3); IRB.CreateCall(TF.TT.TaintSetLabelFn, {Init, - IRB.CreateBitCast(&I, Type::getInt8PtrTy(*TF.TT.Ctx)), + IRB.CreateBitCast(&I, PointerType::getUnqual(*TF.TT.Ctx)), Size}); } } @@ -3750,7 +3813,7 @@ void TaintVisitor::visitMemSetInst(MemSetInst &I) { IRB.CreateCall( TF.TT.TaintSetLabelFn, {ValShadow, - IRB.CreateBitCast(I.getDest(), Type::getInt8PtrTy(*TF.TT.Ctx)), + IRB.CreateBitCast(I.getDest(), PointerType::getUnqual(*TF.TT.Ctx)), IRB.CreateZExtOrTrunc(I.getLength(), TF.TT.IntptrTy)}); } @@ -3770,15 +3833,15 @@ void TaintVisitor::visitMemTransferInst(MemTransferInst &I) { Value *LenShadow = IRB.CreateMul( I.getLength(), ConstantInt::get(I.getLength()->getType(), TF.TT.ShadowWidthBytes)); - Type *Int8Ptr = Type::getInt8PtrTy(*TF.TT.Ctx); + Type *Int8Ptr = PointerType::getUnqual(*TF.TT.Ctx); DestShadow = IRB.CreateBitCast(DestShadow, Int8Ptr); SrcShadow = IRB.CreateBitCast(SrcShadow, Int8Ptr); auto *MTI = cast( IRB.CreateCall(I.getFunctionType(), I.getCalledOperand(), {DestShadow, SrcShadow, LenShadow, I.getVolatileCst()})); if (ClPreserveAlignment) { - MTI->setDestAlignment(I.getDestAlign() * TF.TT.ShadowWidthBytes); - MTI->setSourceAlignment(I.getSourceAlign() * TF.TT.ShadowWidthBytes); + MTI->setDestAlignment(((I.getDestAlign()) ? MaybeAlign(Align((I.getDestAlign())->value() * (uint64_t)(TF.TT.ShadowWidthBytes))) : MaybeAlign())); + MTI->setSourceAlignment(((I.getSourceAlign()) ? MaybeAlign(Align((I.getSourceAlign())->value() * (uint64_t)(TF.TT.ShadowWidthBytes))) : MaybeAlign())); } else { MTI->setDestAlignment(Align(TF.TT.ShadowWidthBytes)); MTI->setSourceAlignment(Align(TF.TT.ShadowWidthBytes)); @@ -3872,14 +3935,17 @@ bool TaintVisitor::visitWrappedCallBase(Function *F, CallBase &CB) { CB.setCalledFunction(F); IRB.CreateCall(TF.TT.TaintUnimplementedFn, IRB.CreateGlobalStringPtr(F->getName())); + TF.TT.buildExternWeakCheckIfNeeded(IRB, F); TF.setShadow(&CB, TF.TT.getZeroShadow(&CB)); return true; case Taint::WK_Discard: CB.setCalledFunction(F); + TF.TT.buildExternWeakCheckIfNeeded(IRB, F); TF.setShadow(&CB, TF.TT.getZeroShadow(&CB)); return true; case Taint::WK_Functional: CB.setCalledFunction(F); + TF.TT.buildExternWeakCheckIfNeeded(IRB, F); //FIXME: // visitOperandShadowInst(CS); return true; @@ -4114,22 +4180,9 @@ bool TaintVisitor::visitWrappedCallBase(Function *F, CallBase &CB) { // Adds non-variable arguments. auto *I = CB.arg_begin(); for (unsigned N = FT->getNumParams(); N != 0; ++I, --N) { - Type *T = (*I)->getType(); - FunctionType *ParamFT; - if (isa(T) && - (ParamFT = dyn_cast(T->getPointerElementType()))) { - std::string TName = "dfst"; - TName += utostr(FT->getNumParams() - N); - TName += "$"; - TName += F->getName(); - Constant *Trampoline = - TF.TT.getOrBuildTrampolineFunction(ParamFT, TName); - Args.push_back(Trampoline); - Args.push_back( - IRB.CreateBitCast(*I, Type::getInt8PtrTy(*TF.TT.Ctx))); - } else { - Args.push_back(*I); - } + // Opaque pointers: cannot recover a function pointee type, so custom- + // wrapper trampolines are gone; pass the argument through unchanged. + Args.push_back(*I); } // Adds shadow arguments. @@ -4175,10 +4228,10 @@ bool TaintVisitor::visitWrappedCallBase(Function *F, CallBase &CB) { void TaintVisitor::visitIntrinsicCallBase(Function *F, CallBase &CB) { // filter some obvious ones StringRef FN = F->getName(); - if (FN.startswith("llvm.va_") || // varabile length - FN.startswith("llvm.gc") || // garbaage collection - FN.startswith("llvm.experimental") || - FN.startswith("llvm.lifetime") + if ((FN).starts_with("llvm.va_") || // varabile length + (FN).starts_with("llvm.gc") || // garbaage collection + (FN).starts_with("llvm.experimental") || + (FN).starts_with("llvm.lifetime") ) { return; } diff --git a/instrumentation/UCSanPass.cpp b/instrumentation/UCSanPass.cpp index 6a16c5fa..b12624a4 100644 --- a/instrumentation/UCSanPass.cpp +++ b/instrumentation/UCSanPass.cpp @@ -49,7 +49,9 @@ #include "UCSanSummary.h" -#include "llvm/ADT/None.h" +#include +#include "llvm/IR/AttributeMask.h" +#include "llvm/TargetParser/Triple.h" #include "llvm/IR/Module.h" #include "llvm/IR/Function.h" #include "llvm/IR/Instructions.h" @@ -87,27 +89,25 @@ #include "llvm/BinaryFormat/Dwarf.h" #include "llvm/Support/JSON.h" #include "llvm/Support/raw_ostream.h" +#include "llvm/Demangle/Demangle.h" + #include #include #include #include #include -#include using namespace llvm; // Demangle a C++ mangled name. If demangling fails (e.g. plain C name), // return the original name unchanged. +// +// Use LLVM's own demangler rather than /abi::__cxa_demangle: when +// building against the LLVM tree, -I/include makes resolve to +// libc++abi's copy, which clashes with the host libstdc++ headers. static std::string demangleName(StringRef MangledName) { - int Status = -1; - char *Demangled = abi::__cxa_demangle( - MangledName.str().c_str(), nullptr, nullptr, &Status); - if (Status == 0 && Demangled) { - std::string Result(Demangled); - free(Demangled); - return Result; - } - return MangledName.str(); + // llvm::demangle returns a copy of the input if no demangling occurred. + return llvm::demangle(MangledName.str()); } namespace { @@ -288,7 +288,7 @@ TransformFunctionAttributes(const TransformedFunction& TransformedFunction, return AttributeList::get(Ctx, CallSiteAttrs.getFnAttrs(), CallSiteAttrs.getRetAttrs(), - llvm::makeArrayRef(ArgumentAttributes)); + ArrayRef(ArgumentAttributes)); } // YAML structures for metadata parsing @@ -485,6 +485,10 @@ class UCSan { std::map TypeIDMap; // type key -> type_id std::map TypeIDToType; // type_id -> LLVM Type* std::map LoadedTypes; // type_id -> JSON from previous modules + // type_id -> real pointee type, for synthetic pointer entries minted by + // getOrCreatePointerTypeID(). Opaque pointers make TypeIDToType[ID] just + // "ptr" for these, so the pointee has to be tracked on the side. + std::map TypeIDPointee; // Builtin type IDs (fixed across modules): // 1=i1, 2=i8, 3=i16, 4=i32, 5=i64, 6=float, 7=double static const uint32_t kFirstDynamicTypeID = 16; // IDs 1-15 reserved for builtins @@ -501,6 +505,11 @@ class UCSan { std::vector EntryArgs; uint32_t getOrCreateTypeID(Type *T); + // Mints (or reuses) a type_id for "pointer to PointeeTy". Opaque pointers + // collapse every PointerType to the same Type object, so this can't key + // off Type* like getOrCreateTypeID() does — it keys off the pointee's own + // key instead, keeping distinct pointees distinguishable. + uint32_t getOrCreatePointerTypeID(Type *PointeeTy); std::string getTypeKey(Type *T); void emitTypeTable(); void loadTypeTable(); @@ -561,7 +570,7 @@ class UCSan { /// Marks an instruction as "nosanitize" so TaintPass will skip it inline void markNosanitize(Value *V) { Instruction *I = dyn_cast(V); - if (I) I->setMetadata("nosanitize", MDNode::get(*Ctx, None)); + if (I) I->setMetadata("nosanitize", MDNode::get(*Ctx, std::nullopt)); } /// Marks a function as "nosanitize" so TaintPass will skip instrumenting it @@ -695,6 +704,7 @@ class UCSanVisitor : public InstVisitor { void visitBinaryOperator(BinaryOperator &BO); void visitCmpInst(CmpInst &CI); void visitAtomicRMWInst(AtomicRMWInst &I); + void visitAtomicCmpXchgInst(AtomicCmpXchgInst &I); void visitLoadInst(LoadInst &LI); void visitStoreInst(StoreInst &SI); void visitMemCpyInst(MemCpyInst &I); @@ -1055,6 +1065,65 @@ uint32_t UCSan::getOrCreateTypeID(Type *T) { return ID; } +// Opaque pointers erase pointee type from a pointer Value's Type, but GEP +// instructions still carry an explicit SourceElementType (they need it to +// compute the byte offset). So find the pointee type by walking forward +// through V's uses: a GEP that takes V as its base pointer operand reveals +// what V conceptually points to. Chases through PHI nodes (e.g. the loop +// variable in `for (n = head; n; n = n->next)`) since a GEP consuming a +// loaded/argument pointer is often one hop removed via a PHI rather than a +// direct use. Purely a compile-time analysis, so an exhaustive walk (bounded +// by the Visited set against PHI cycles) is affordable. +static Type *findPointeeTypeFromUses(Value *V, SmallPtrSetImpl &Visited) { + if (!Visited.insert(V).second) + return nullptr; + for (User *U : V->users()) { + if (auto *GEP = dyn_cast(U)) { + if (GEP->getPointerOperand() == V) + return GEP->getSourceElementType(); + } else if (auto *PN = dyn_cast(U)) { + if (Type *Found = findPointeeTypeFromUses(PN, Visited)) + return Found; + } + } + return nullptr; +} + +static Type *findPointeeTypeFromUses(Value *V) { + SmallPtrSet Visited; + return findPointeeTypeFromUses(V, Visited); +} + +// Opaque pointers erase pointee type going forward too: a pointer Value's own +// Type carries nothing. For call sites that need *some* size/type for a +// pointer whose uses don't reveal it (it's about to be handed to opaque code +// - an indirect call, inline asm, a custom/wrapped function), fall back to +// tracing backward to the concrete allocation it originates from. alloca and +// global declarations always carry their real allocated type regardless of +// pointer opacity. Returns nullptr (unknown) for anything else, e.g. a heap +// pointer or a plain incoming argument with no local origin. +static Type *getUnderlyingObjectType(Value *Ptr) { + Value *Obj = getUnderlyingObject(Ptr); + if (auto *AI = dyn_cast(Obj)) + return AI->getAllocatedType(); + if (auto *GV = dyn_cast(Obj)) + return GV->getValueType(); + return nullptr; +} + +uint32_t UCSan::getOrCreatePointerTypeID(Type *PointeeTy) { + std::string Key = "ptr->" + getTypeKey(PointeeTy); + auto It = TypeIDMap.find(Key); + if (It != TypeIDMap.end()) + return It->second; + uint32_t ID = NextTypeID++; + TypeIDMap[Key] = ID; + TypeIDToType[ID] = VoidPtrTy; + TypeIDPointee[ID] = PointeeTy; + getOrCreateTypeID(PointeeTy); // ensure the pointee itself has an entry too + return ID; +} + void UCSan::loadTypeTable() { if (ClTypeTable.empty()) return; @@ -1195,11 +1264,15 @@ void UCSan::emitTypeTable() { TypeObj["element_type_id"] = getOrCreateTypeID(AT->getElementType()); if (T->isSized()) TypeObj["size"] = (int64_t)DL.getTypeAllocSize(T); - } else if (auto *PT = dyn_cast(T)) { + } else if (isa(T)) { TypeObj["kind"] = "pointer"; TypeObj["size"] = (int64_t)DL.getTypeAllocSize(T); - Type *PointeeTy = PT->getPointerElementType(); - if (PointeeTy->isSized()) + // Opaque pointers carry no pointee info in T itself. Entries minted by + // getOrCreatePointerTypeID() record the real pointee on the side; + // anything else is a generic/unknown pointer (pointee_type_id 0). + auto PointeeIt = TypeIDPointee.find(ID); + Type *PointeeTy = PointeeIt != TypeIDPointee.end() ? PointeeIt->second : nullptr; + if (PointeeTy && PointeeTy->isSized()) TypeObj["pointee_type_id"] = getOrCreateTypeID(PointeeTy); else TypeObj["pointee_type_id"] = 0; @@ -1451,7 +1524,7 @@ Value *UCSanFunction::loadShadowRecursive( if (!isa(SubTy) && !isa(SubTy)) { uint64_t SubSize = DL.getTypeStoreSize(SubTy); assert(Size >= SubSize); - InstAlign = Align(std::min(InstAlign.value(), (uint64_t)DL.getABITypeAlignment(SubTy))); + InstAlign = Align(std::min(InstAlign.value(), (uint64_t)(DL).getABITypeAlign(SubTy).value())); // load a primitive shadow from address Value *PrimitiveShadow = loadPrimitiveShadow(Addr, SubSize, InstAlign, SubTy, IRB); // then insert the primitive shadow into the sub-field @@ -1550,7 +1623,7 @@ void UCSanFunction::storeShadowRecursive( uint64_t SubSize = DL.getTypeStoreSize(SubTy); assert(Size >= SubSize); InstAlign = Align(std::min(InstAlign.value(), - (uint64_t)DL.getABITypeAlignment(SubTy))); + (uint64_t)(DL).getABITypeAlign(SubTy).value())); // load a primitive shadow from the sub-field Value *PrimitiveShadow = IRB.CreateExtractValue(Shadow, Indices); UC.markNosanitize(PrimitiveShadow); @@ -1706,13 +1779,11 @@ Function *UCSan::buildDangleFunction(Function *F) { for (auto arg = NewF->arg_begin(); n != 0; ++arg, --n) { if (arg->getType()->isPointerTy()) { - // Get size of pointed-to type - ConstantInt *CI; - if (arg->getType()->getPointerElementType()->isSized()) { - CI = ConstantInt::get(Int64Ty, DL.getTypeSizeInBits(arg->getType()->getPointerElementType()) / 8); - } else { - CI = ConstantInt::get(Int64Ty, 0); - } + // F is an out-of-scope declaration (that's the point of a dangle + // wrapper) with no body, and arg is a fresh stub argument with no + // uses of its own - there's no pointee type to recover here under + // opaque pointers, so treat the pointed-to size as unknown. + ConstantInt *CI = ConstantInt::get(Int64Ty, 0); // Get shadow address for this argument from TLS // UCSan uses shadow memory to track pointer aliasing @@ -1881,8 +1952,13 @@ Function *UCSan::buildDriverWrapperFunction(Function *F) { ConstantInt *IsPtr = ConstantInt::get(Int8Ty, ai->getType()->isPointerTy()); ConstantInt *InitVal = ConstantInt::get(Int64Ty, 0); - // Record arg type info - uint32_t TypeID = getOrCreateTypeID(ai->getType()); + // Record arg type info. Entry args are what unittest-style seed gen + // needs real types for, so recover pointer args' pointee type the same + // way as pointer-typed loads: from how the argument gets used (e.g. a + // GEP indexing into it) rather than from its now-opaque Type. + Type *PointeeTy = ai->getType()->isPointerTy() ? findPointeeTypeFromUses(&*ai) : nullptr; + uint32_t TypeID = PointeeTy ? getOrCreatePointerTypeID(PointeeTy) + : getOrCreateTypeID(ai->getType()); EntryArgs.push_back({ai->getName().str(), TypeID}); // Call runtime to create symbolic argument @@ -2529,7 +2605,15 @@ void UCSanVisitor::visitLoadInst(LoadInst &LI) { } } ConstantInt *Size = ConstantInt::get(UF.UC.Int64Ty, StoreSize); - uint32_t TypeID = UF.UC.getOrCreateTypeID(Ty); + // Opaque pointers make Ty uninformative when the loaded value is itself a + // pointer (e.g. a linked-list `next` field) - every such load would + // otherwise collapse onto the same generic "ptr" type_id. Recover the real + // pointee by looking at how the loaded pointer gets used afterward (e.g. a + // GEP indexing into it), so the lazily-created target object can be sized + // correctly up front instead of growing it one field access at a time. + Type *PointeeTy = Ty->isPointerTy() ? findPointeeTypeFromUses(&LI) : nullptr; + uint32_t TypeID = PointeeTy ? UF.UC.getOrCreatePointerTypeID(PointeeTy) + : UF.UC.getOrCreateTypeID(Ty); // Check and replace pointer IRBuilder<> IRB(&LI); @@ -2544,7 +2628,7 @@ void UCSanVisitor::visitLoadInst(LoadInst &LI) { // Mark as checked for TaintPass LI.setMetadata("ucsan.checked", - MDNode::get(*UF.UC.Ctx, None)); + MDNode::get(*UF.UC.Ctx, std::nullopt)); } void UCSanVisitor::visitStoreInst(StoreInst &SI) { @@ -2570,7 +2654,7 @@ void UCSanVisitor::visitStoreInst(StoreInst &SI) { // Mark as checked SI.setMetadata("ucsan.checked", - MDNode::get(*UF.UC.Ctx, None)); + MDNode::get(*UF.UC.Ctx, std::nullopt)); } void UCSanVisitor::visitMemCpyInst(MemCpyInst &I) { @@ -2607,7 +2691,7 @@ void UCSanVisitor::visitMemCpyInst(MemCpyInst &I) { I.getVolatileCst()}); UF.UC.markNosanitize(CI); - I.setMetadata("ucsan.checked", MDNode::get(*UF.UC.Ctx, None)); + I.setMetadata("ucsan.checked", MDNode::get(*UF.UC.Ctx, std::nullopt)); } void UCSanVisitor::visitMemSetInst(MemSetInst &I) { @@ -2632,7 +2716,7 @@ void UCSanVisitor::visitMemSetInst(MemSetInst &I) { Value *CI = IRB.CreateCall(UF.UC.UCSetLabelFn, {ValShadow, dest, Length}); UF.UC.markNosanitize(CI); - I.setMetadata("ucsan.checked", MDNode::get(*UF.UC.Ctx, None)); + I.setMetadata("ucsan.checked", MDNode::get(*UF.UC.Ctx, std::nullopt)); } void UCSanVisitor::visitMemMoveInst(MemMoveInst &I) { @@ -2671,7 +2755,7 @@ void UCSanVisitor::visitMemMoveInst(MemMoveInst &I) { MTI->setSourceAlignment(Align(UF.UC.ShadowWidthBytes)); UF.UC.markNosanitize(MTI); - I.setMetadata("ucsan.checked", MDNode::get(*UF.UC.Ctx, None)); + I.setMetadata("ucsan.checked", MDNode::get(*UF.UC.Ctx, std::nullopt)); } void UCSanVisitor::visitGetElementPtrInst(GetElementPtrInst &GEPI) { @@ -2724,9 +2808,11 @@ void UCSanVisitor::visitInlineAsm(InlineAsm *IA, CallBase &CB) { if (!isa(Arg) && !isa(Arg->stripPointerCasts())) { unsigned ObjSize = 0; - Type *PointeeTy = Arg->getType()->getPointerElementType(); + // AllocaInst is excluded above, so this traces heap/global origins; + // opaque pointers carry no pointee type on Arg itself. + Type *PointeeTy = getUnderlyingObjectType(Arg); uint32_t TypeID = 0; - if (PointeeTy->isSized()) { + if (PointeeTy && PointeeTy->isSized()) { ObjSize = DL.getTypeAllocSize(PointeeTy); TypeID = UF.UC.getOrCreateTypeID(PointeeTy); } @@ -3018,13 +3104,12 @@ void UCSanVisitor::visitIndirectCallBase(Value *FPtr, CallBase &CB) { if (getenv("KO_RESIGN_PTRARGS") && Arg->getType()->isPointerTy()) { // only resign ptr args std::vector Args; - ConstantInt *CI; - if (Arg->getType()->getPointerElementType()->isSized()) { - CI = ConstantInt::get(UF.UC.Int64Ty, - DL.getTypeSizeInBits(Arg->getType()->getPointerElementType()) / 8); - } else { - CI = ConstantInt::get(UF.UC.Int64Ty, 0); - } + // Resign the whole underlying object (alloca/global) the pointer + // points into, since opaque pointers no longer carry pointee type. + Type *PointeeTy = getUnderlyingObjectType(Arg); + uint64_t ObjSize = (PointeeTy && PointeeTy->isSized()) + ? DL.getTypeAllocSize(PointeeTy) : 0; + ConstantInt *CI = ConstantInt::get(UF.UC.Int64Ty, ObjSize); auto BCI = IRB_EB.CreateBitCast(Arg, UF.UC.VoidPtrTy); // FIXME: cast to i32* (defined as void *) auto ArgTLS = UF.getArgTLS(Arg->getType(), ArgOffset, IRB_EB); Args.push_back(BCI); @@ -3232,9 +3317,15 @@ bool UCSanVisitor::visitWrappedCallBase(Function *F, CallBase &CB) { sizeArg = ConstantInt::get(UF.UC.Int64Ty, 0); // string fn, unknown length } } else { - Type *PointeeTy = (*I)->getType()->getPointerElementType(); - sizeArg = ConstantInt::get(UF.UC.Int64Ty, DL.getTypeAllocSize(PointeeTy)); - TypeID = UF.UC.getOrCreateTypeID(PointeeTy); + // Opaque pointers carry no pointee type on (*I) itself; trace back + // to its underlying allocation instead. + Type *PointeeTy = getUnderlyingObjectType(*I); + if (PointeeTy && PointeeTy->isSized()) { + sizeArg = ConstantInt::get(UF.UC.Int64Ty, DL.getTypeAllocSize(PointeeTy)); + TypeID = UF.UC.getOrCreateTypeID(PointeeTy); + } else { + sizeArg = ConstantInt::get(UF.UC.Int64Ty, 0); + } } // Capture shadow before checkPointer resolves the pointer @@ -3343,7 +3434,7 @@ bool UCSanVisitor::visitWrappedCallBase(Function *F, CallBase &CB) { return false; auto FName = F->getName(); - bool IsContractPrim = FName.startswith("assume_") || FName.startswith("assert_"); + bool IsContractPrim = (FName).starts_with("assume_") || (FName).starts_with("assert_"); TransformedFunction CustomFn = UF.UC.getCustomFunctionType(FT); std::string CustomFName = "__dfsw_" + FName.str(); @@ -3367,13 +3458,18 @@ bool UCSanVisitor::visitWrappedCallBase(Function *F, CallBase &CB) { for (unsigned N = FT->getNumParams(); N != 0; ++I, --N) { Type *T = (*I)->getType(); if (isa(T) && !IsContractPrim) { - // Check pointer arguments before passing to custom function + // Check pointer arguments before passing to custom function. + // Opaque pointers carry no pointee type on T itself; trace back + // to the underlying allocation instead. auto DL = getDataLayout(); - Type *PointeeTy = T->getPointerElementType(); - Value *sizeArg = - ConstantInt::get(UF.UC.Int64Ty, - DL.getTypeAllocSize(PointeeTy)); - uint32_t TypeID = UF.UC.getOrCreateTypeID(PointeeTy); + Type *PointeeTy = getUnderlyingObjectType(*I); + uint32_t TypeID = 0; + uint64_t PointeeSize = 0; + if (PointeeTy && PointeeTy->isSized()) { + PointeeSize = DL.getTypeAllocSize(PointeeTy); + TypeID = UF.UC.getOrCreateTypeID(PointeeTy); + } + Value *sizeArg = ConstantInt::get(UF.UC.Int64Ty, PointeeSize); Value *rptr = UF.checkPointer(*I, sizeArg, true, IRB, TypeID); Args.push_back(rptr); } else { @@ -3495,8 +3591,8 @@ void UCSanVisitor::visitCallBase(CallBase &CB) { const DataLayout &DL = getDataLayout(); // Stores argument shadows. - if (F && F->hasName() && !F->getName().startswith("__dfsan") && - !F->getName().startswith("__taint")) { + if (F && F->hasName() && !(F->getName()).starts_with("__dfsan") && + !(F->getName()).starts_with("__taint")) { unsigned ArgOffset = 0; for (unsigned I = 0, N = FT->getNumParams(); I != N; ++I) { unsigned Size = @@ -3605,7 +3701,26 @@ void UCSanVisitor::visitAtomicRMWInst(AtomicRMWInst &I) { // FIXME: AtomicRMWInst should not operate on ptrs UF.setShadow(&I, UF.UC.ZeroPrimitiveShadow); - I.setMetadata("ucsan.checked", MDNode::get(*UF.UC.Ctx, None)); + I.setMetadata("ucsan.checked", MDNode::get(*UF.UC.Ctx, std::nullopt)); +} + +void UCSanVisitor::visitAtomicCmpXchgInst(AtomicCmpXchgInst &I) { + auto &DL = I.getModule()->getDataLayout(); + Value *Ptr = I.getPointerOperand(); + Type *Ty = I.getNewValOperand()->getType(); + unsigned StoreSize = DL.getTypeStoreSize(Ty); + ConstantInt *Size = ConstantInt::get(UF.UC.Int64Ty, StoreSize); + uint32_t TypeID = UF.UC.getOrCreateTypeID(Ty); + + IRBuilder<> IRB(&I); + Ptr = UF.checkPointer(Ptr, Size, true, IRB, TypeID); + I.setOperand(0, Ptr); + + // The result is { Ty old_value, i1 success }; like AtomicRMWInst we only + // bounds-check the pointer here and do not propagate a symbolic result. + UF.setShadow(&I, UF.UC.getZeroShadow(&I)); + + I.setMetadata("ucsan.checked", MDNode::get(*UF.UC.Ctx, std::nullopt)); } void UCSanVisitor::visitAllocaInst(AllocaInst &I) { @@ -3642,7 +3757,7 @@ void UCSanVisitor::visitAllocaInst(AllocaInst &I) { IRBuilder<> IRB(I.getNextNode()); auto DL = I.getModule()->getDataLayout(); auto allocaSizeInBits = I.getAllocationSizeInBits(DL); - if (allocaSizeInBits.hasValue()) { + if (allocaSizeInBits) { int allocaSizeInBytes = (allocaSizeInBits->getFixedValue() + 7) >> 3; Value* Size = ConstantInt::get(UF.UC.Int64Ty, allocaSizeInBytes); Value* Ptr = IRB.CreateBitOrPointerCast(&I, UF.UC.VoidPtrTy); @@ -3770,7 +3885,7 @@ void UCSanVisitor::visitPHINode(PHINode &PN) { // Give the shadow phi node valid predecessors to fool SplitEdge into working. Value *UndefShadow = UndefValue::get(ShadowTy); - for (PHINode::block_iterator i = PN.block_begin(), e = PN.block_end(); i != e; + for (auto i = PN.block_begin(), e = PN.block_end(); i != e; ++i) { ShadowPN->addIncoming(UndefShadow, *i); } diff --git a/libcxx/build_native/lib/libc++abi.a b/libcxx/build_native/lib/libc++abi.a index 55f18733..c3212570 100644 Binary files a/libcxx/build_native/lib/libc++abi.a and b/libcxx/build_native/lib/libc++abi.a differ diff --git a/libcxx/build_native/lib/libunwind.a b/libcxx/build_native/lib/libunwind.a index e5cd2922..d073f276 100644 Binary files a/libcxx/build_native/lib/libunwind.a and b/libcxx/build_native/lib/libunwind.a differ diff --git a/libcxx/build_taint/lib/libc++.a b/libcxx/build_taint/lib/libc++.a index d1b39c8f..4d4c67d9 100644 Binary files a/libcxx/build_taint/lib/libc++.a and b/libcxx/build_taint/lib/libc++.a differ diff --git a/libcxx/build_taint/lib/libc++abi.a b/libcxx/build_taint/lib/libc++abi.a index 1831ffa6..ea3e442a 100644 Binary files a/libcxx/build_taint/lib/libc++abi.a and b/libcxx/build_taint/lib/libc++abi.a differ diff --git a/libcxx/build_taint/lib/libunwind.a b/libcxx/build_taint/lib/libunwind.a index a71f6f63..537d0f49 100644 Binary files a/libcxx/build_taint/lib/libunwind.a and b/libcxx/build_taint/lib/libunwind.a differ diff --git a/libcxx/rebuild.sh b/libcxx/rebuild.sh index e2439533..bb094f55 100755 --- a/libcxx/rebuild.sh +++ b/libcxx/rebuild.sh @@ -22,7 +22,20 @@ if [ ! -h $CXX ]; then exit 1 fi -LLVM_VERSION=14.0.6 +# The underlying (non-instrumented) clang that ko-clang wraps. We are migrating +# to LLVM 18, so clang-18 is the default; override via the environment to build +# against a different LLVM (e.g. KO_CC=clang-14). +export KO_CC=${KO_CC:-clang-18} +export KO_CXX=${KO_CXX:-clang++-18} + +# Derive the LLVM version to check out from the compiler itself so the libc++ +# sources always match the toolchain doing the instrumentation. +LLVM_VERSION=$(${KO_CC} --version | sed -n 's/.*clang version \([0-9][0-9.]*\).*/\1/p' | head -1) +if [ -z "$LLVM_VERSION" ]; then + echo "[-] Error: could not determine LLVM version from '${KO_CC}'" 1>&2 + exit 1 +fi +LLVM_MAJOR=${LLVM_VERSION%%.*} NINJA_B=`which ninja 2>/dev/null` @@ -35,18 +48,27 @@ fi set -euxo pipefail CUR_DIR=`pwd` -LLVM_SRC="llvm_project" +# Keep a per-major-version source tree so multiple LLVM versions can coexist. +LLVM_SRC="llvm_project-${LLVM_MAJOR}" if [ ! -d $LLVM_SRC ]; then git clone --depth 1 --branch llvmorg-${LLVM_VERSION} https://github.com/llvm/llvm-project.git $LLVM_SRC fi +# LLVM 16+ libunwind prefers glibc's _dl_find_object over dl_iterate_phdr to +# locate EH frames. _dl_find_object bypasses DFSan's dl_iterate_phdr wrapper and +# cannot resolve unwind info for symsan's fixed-address (taint.ld) binaries, +# which breaks C++ exception handling. Force the dl_iterate_phdr path. +# Idempotent: rewrites only the pristine guard line. +ASPACE="$LLVM_SRC/libunwind/src/AddressSpace.hpp" +if [ -f "$ASPACE" ]; then + sed -i 's|^#if defined(DLFO_STRUCT_HAS_EH_DBASE) & defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)|#if 0 /* symsan: force dl_iterate_phdr; _dl_find_object bypasses DFSan */ \&\& defined(DLFO_STRUCT_HAS_EH_DBASE) \& defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)|' "$ASPACE" +fi + mkdir -p build_taint rm -rf build_taint/* export KO_CONFIG=1 -export KO_CC=clang-14 -export KO_CXX=clang++-14 cmake -G Ninja -S $LLVM_SRC/runtimes -B build_taint \ -DLLVM_TARGETS_TO_BUILD=X86 -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_C_COMPILER=${CC} -DCMAKE_CXX_COMPILER=${CXX} \ diff --git a/libcxx/rebuild_native.sh b/libcxx/rebuild_native.sh index ee88b406..3503ea3a 100755 --- a/libcxx/rebuild_native.sh +++ b/libcxx/rebuild_native.sh @@ -19,12 +19,20 @@ # out-of-scope and dangled by UCSan. # # usage: rebuild_native.sh -# Override the compiler with KO_NATIVE_CC / KO_NATIVE_CXX (default clang-14). +# Override the compiler with KO_NATIVE_CC / KO_NATIVE_CXX (default clang-18). -LLVM_VERSION=14.0.6 +# We are migrating to LLVM 18, so clang-18 is the default. +CC=${KO_NATIVE_CC:-clang-18} +CXX=${KO_NATIVE_CXX:-clang++-18} -CC=${KO_NATIVE_CC:-clang-14} -CXX=${KO_NATIVE_CXX:-clang++-14} +# Derive the LLVM version to check out from the compiler itself so the EH +# runtime sources match the toolchain. +LLVM_VERSION=$(${CC} --version | sed -n 's/.*clang version \([0-9][0-9.]*\).*/\1/p' | head -1) +if [ -z "$LLVM_VERSION" ]; then + echo "[-] Error: could not determine LLVM version from '${CC}'" 1>&2 + exit 1 +fi +LLVM_MAJOR=${LLVM_VERSION%%.*} NINJA_B=`which ninja 2>/dev/null` @@ -37,12 +45,25 @@ fi set -euxo pipefail CUR_DIR=`pwd` -LLVM_SRC="llvm_project" +# Keep a per-major-version source tree so multiple LLVM versions can coexist +# (shared with rebuild.sh). +LLVM_SRC="llvm_project-${LLVM_MAJOR}" if [ ! -d $LLVM_SRC ]; then git clone --depth 1 --branch llvmorg-${LLVM_VERSION} https://github.com/llvm/llvm-project.git $LLVM_SRC fi +# LLVM 16+ libunwind prefers glibc's _dl_find_object over dl_iterate_phdr to +# locate EH frames. _dl_find_object bypasses DFSan's dl_iterate_phdr wrapper and +# cannot resolve unwind info for symsan's fixed-address (taint.ld) binaries, +# which breaks C++ exception handling. Force the dl_iterate_phdr path. +# Idempotent: rewrites only the pristine guard line. (Shared source tree with +# rebuild.sh, so whichever runs first applies it and the other is a no-op.) +ASPACE="$LLVM_SRC/libunwind/src/AddressSpace.hpp" +if [ -f "$ASPACE" ]; then + sed -i 's|^#if defined(DLFO_STRUCT_HAS_EH_DBASE) & defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)|#if 0 /* symsan: force dl_iterate_phdr; _dl_find_object bypasses DFSan */ \&\& defined(DLFO_STRUCT_HAS_EH_DBASE) \& defined(_LIBUNWIND_SUPPORT_DWARF_INDEX)|' "$ASPACE" +fi + mkdir -p build_native rm -rf build_native/* diff --git a/parsers/rgd-parser.cpp b/parsers/rgd-parser.cpp index f7606070..8e607984 100644 --- a/parsers/rgd-parser.cpp +++ b/parsers/rgd-parser.cpp @@ -306,13 +306,21 @@ bool RGDAstParser::do_uta_rel(dfsan_label label, rgd::AstNode *ret, return false; } dfsan_label_info *src = get_label_info(info->l2); - if (unlikely(src->op != __dfsan::Load)) { + // The consumed digits are represented either as a Load label (len > 1) or, + // when a single digit was consumed, as the raw input-byte label directly + // (op == 0). + dfsan_label_info *byte; + if (src->op == __dfsan::Load) { + byte = get_label_info(src->l1); + } else if (src->op == 0) { + byte = src; + } else { WARNF("invalid atoi source label %u, op = %u\n", info->l2, src->op); return false; } visited.insert(info->l2); - uint32_t input_id = get_label_info(src->l1)->op2.i; - uint32_t offset = get_label_info(src->l1)->op1.i; + uint32_t input_id = byte->op2.i; + uint32_t offset = byte->op1.i; // this check should have been done during label scanning // if (unlikely(offset >= buf_size)) { // WARNF("invalid offset: %lu >= %lu\n", offset, buf_size); diff --git a/runtime/dfsan/dfsan.cpp b/runtime/dfsan/dfsan.cpp index 8974aa67..7d7cc42c 100644 --- a/runtime/dfsan/dfsan.cpp +++ b/runtime/dfsan/dfsan.cpp @@ -1179,6 +1179,17 @@ void __dfsan_unimplemented(char *fname) { } +extern "C" SANITIZER_INTERFACE_ATTRIBUTE void __dfsan_wrapper_extern_weak_null( + const void *addr, char *fname) { + if (!addr) + Report( + "ERROR: DataFlowSanitizer: dfsan generated wrapper calling null " + "extern_weak function %s\nIf this only happens with dfsan, the " + "dfsan instrumentation pass may be accidentally optimizing out a " + "null check\n", + fname); +} + // Use '-mllvm -dfsan-debug-nonzero-labels' and break on this function // to try to figure out where labels are being introduced in a nominally // label-free program. @@ -2040,6 +2051,16 @@ void __taint_set_retval_tls(uint32_t index, dfsan_label label, uint32_t size_in_ __dfsan_retval_tls[index] = label; } +// Zero the argument/return-value TLS. Custom function wrappers that invoke an +// instrumented callback directly (e.g. dl_iterate_phdr, pthread_create) use +// this to give the callback zero-labelled arguments now that the trampoline +// mechanism has been removed (opaque pointers, LLVM 15+). +SANITIZER_INTERFACE_ATTRIBUTE +void dfsan_clear_thread_local_state() { + internal_memset(__dfsan_arg_tls, 0, sizeof(__dfsan_arg_tls)); + internal_memset(__dfsan_retval_tls, 0, sizeof(__dfsan_retval_tls)); +} + // Set SymSan shadow memory for a region // Overrides weak stub in ucsan.cpp SANITIZER_INTERFACE_ATTRIBUTE diff --git a/runtime/dfsan/dfsan.h b/runtime/dfsan/dfsan.h index 300a9a9d..916c64a5 100644 --- a/runtime/dfsan/dfsan.h +++ b/runtime/dfsan/dfsan.h @@ -84,6 +84,7 @@ struct taint_socket { extern "C" { void dfsan_add_label(dfsan_label label, uint8_t op, void *addr, uptr size); void dfsan_set_label(dfsan_label label, void *addr, uptr size); +void dfsan_clear_thread_local_state(); dfsan_label dfsan_read_label(const void *addr, uptr size); void dfsan_store_label(dfsan_label l1, void *addr, uptr size); dfsan_label dfsan_union(dfsan_label l1, dfsan_label l2, uint16_t op, uint16_t size, @@ -205,6 +206,15 @@ enum operators { LastOp = last_llvm_op + 22, // 89 }; +// Flag packed into the high bits of a fatoi label's op1 (which otherwise holds +// the numeric base). When set, the solver must NOT append a NUL terminator +// after the rendered digits: the parsed number is embedded in a larger input +// (e.g. an sscanf field) rather than a standalone null-terminated string, so a +// NUL would clobber the following separator/bytes. Kept clear for +// atoi/strtol so their labels stay bit-identical. +#define FATOI_NO_NULL (1u << 16) +#define FATOI_BASE_MASK 0xffffu + enum predicate { bveq = 32, bvneq = 33, diff --git a/runtime/dfsan/dfsan_custom.cpp b/runtime/dfsan/dfsan_custom.cpp index c545a86e..1ed98b5c 100644 --- a/runtime/dfsan/dfsan_custom.cpp +++ b/runtime/dfsan/dfsan_custom.cpp @@ -1397,36 +1397,37 @@ __dfsw_dlopen(const char *filename, int flag, dfsan_label filename_label, dfsan_label flag_label, dfsan_label *ret_label) { void *handle = dlopen(filename, flag); link_map *map = GET_LINK_MAP_BY_DLOPEN_HANDLE(handle); - if (map && map->l_addr) + // dlopen(NULL, ...) returns the main executable's map; don't clear the shadow + // of its already-live globals. (l_addr==0 additionally guards non-PIE mains.) + if (filename && map && map->l_addr) ForEachMappedRegion(map, dfsan_set_zero_label); *ret_label = 0; return handle; } struct pthread_create_info { - void *(*start_routine_trampoline)(void *, void *, dfsan_label, dfsan_label *); - void *start_routine; + void *(*start_routine)(void *); void *arg; }; static void *pthread_create_cb(void *p) { pthread_create_info pci(*(pthread_create_info *)p); free(p); - dfsan_label ret_label; - return pci.start_routine_trampoline(pci.start_routine, pci.arg, 0, - &ret_label); + // Trampolines were removed with opaque pointers (LLVM 15+); the instrumented + // start routine reads its argument label from the args TLS, so clear it to + // give a zero-labelled argument before calling it directly. + dfsan_clear_thread_local_state(); + return pci.start_routine(pci.arg); } SANITIZER_INTERFACE_ATTRIBUTE int __dfsw_pthread_create( pthread_t *thread, const pthread_attr_t *attr, - void *(*start_routine_trampoline)(void *, void *, dfsan_label, - dfsan_label *), - void *start_routine, void *arg, dfsan_label thread_label, + void *(*start_routine)(void *), + void *arg, dfsan_label thread_label, dfsan_label attr_label, dfsan_label start_routine_label, dfsan_label arg_label, dfsan_label *ret_label) { pthread_create_info *pci = (pthread_create_info *)malloc(sizeof(pthread_create_info)); - pci->start_routine_trampoline = start_routine_trampoline; pci->start_routine = start_routine; pci->arg = arg; int rv = pthread_create(thread, attr, pthread_create_cb, (void *)pci); @@ -1449,11 +1450,7 @@ SANITIZER_INTERFACE_ATTRIBUTE int __dfsw_pthread_join(pthread_t thread, } struct dl_iterate_phdr_info { - int (*callback_trampoline)(void *callback, struct dl_phdr_info *info, - size_t size, void *data, dfsan_label info_label, - dfsan_label size_label, dfsan_label data_label, - dfsan_label *ret_label); - void *callback; + int (*callback)(struct dl_phdr_info *info, size_t size, void *data); void *data; }; @@ -1465,19 +1462,18 @@ int dl_iterate_phdr_cb(struct dl_phdr_info *info, size_t size, void *data) { dfsan_set_label( 0, const_cast(reinterpret_cast(info->dlpi_phdr)), sizeof(*info->dlpi_phdr) * info->dlpi_phnum); - dfsan_label ret_label; - return dipi->callback_trampoline(dipi->callback, info, size, dipi->data, 0, 0, - 0, &ret_label); + // The trampoline mechanism was removed with opaque pointers (LLVM 15+); the + // instrumented callback now reads its argument labels from the args TLS, so + // clear it to give the callback zero-labelled arguments before calling it. + dfsan_clear_thread_local_state(); + return dipi->callback(info, size, dipi->data); } SANITIZER_INTERFACE_ATTRIBUTE int __dfsw_dl_iterate_phdr( - int (*callback_trampoline)(void *callback, struct dl_phdr_info *info, - size_t size, void *data, dfsan_label info_label, - dfsan_label size_label, dfsan_label data_label, - dfsan_label *ret_label), - void *callback, void *data, dfsan_label callback_label, - dfsan_label data_label, dfsan_label *ret_label) { - dl_iterate_phdr_info dipi = { callback_trampoline, callback, data }; + int (*callback)(struct dl_phdr_info *info, size_t size, void *data), + void *data, dfsan_label callback_label, dfsan_label data_label, + dfsan_label *ret_label) { + dl_iterate_phdr_info dipi = { callback, data }; *ret_label = 0; return dl_iterate_phdr(dl_iterate_phdr_cb, &dipi); } @@ -1684,7 +1680,12 @@ char *__dfsw_strcpy(char *dest, const char *src, dfsan_label dst_label, return ret; } -static dfsan_label taint_strtol(const char *nptr, uptr len, size_t ret_size, int base) { +// add_null: whether the parsed number is a standalone null-terminated string +// (atoi/strtol) so the solver may append a NUL after the rendered digits. For +// numbers embedded in a larger input (e.g. an sscanf field) pass false so the +// separator/following bytes are preserved. +static dfsan_label taint_strtol(const char *nptr, uptr len, size_t ret_size, + int base, bool add_null = true) { dfsan_label load = 0; if (len > 0) { load = dfsan_read_label(nptr, len); @@ -1695,7 +1696,8 @@ static dfsan_label taint_strtol(const char *nptr, uptr len, size_t ret_size, int return 0; load = dfsan_union(l, 0, Load, 0, 0, 0); } - return dfsan_union(0, load, fatoi, sizeof(ret_size) * 8, base, len); + int op1 = base | (add_null ? 0 : FATOI_NO_NULL); + return dfsan_union(0, load, fatoi, sizeof(ret_size) * 8, op1, len); } SANITIZER_INTERFACE_ATTRIBUTE @@ -1782,9 +1784,9 @@ unsigned long __dfsw_strtoul(const char *nptr, char **endptr, int base, } SANITIZER_INTERFACE_ATTRIBUTE -unsigned long long __dfsw_strtoull(const char *nptr, char **endptr, +unsigned long long __dfsw_strtoull(const char *nptr, char **endptr, int base, dfsan_label nptr_label, - int base, dfsan_label endptr_label, + dfsan_label endptr_label, dfsan_label base_label, dfsan_label *ret_label) { char *tmp_endptr; @@ -1797,6 +1799,48 @@ unsigned long long __dfsw_strtoull(const char *nptr, char **endptr, return ret; } +// glibc 2.38 (C23) redirects strtol/strtoll/strtoul/strtoull in to +// these __isoc23_* variants at compile time, so on modern glibc (Ubuntu 24.04) +// user calls to strtol&co land here. Forward to the symbolic base wrappers so +// the parsed integer's taint is preserved. +SANITIZER_INTERFACE_ATTRIBUTE +long __dfsw___isoc23_strtol(const char *nptr, char **endptr, int base, + dfsan_label nptr_label, dfsan_label endptr_label, + dfsan_label base_label, dfsan_label *ret_label) { + return __dfsw_strtol(nptr, endptr, base, nptr_label, endptr_label, base_label, + ret_label); +} + +SANITIZER_INTERFACE_ATTRIBUTE +long long __dfsw___isoc23_strtoll(const char *nptr, char **endptr, int base, + dfsan_label nptr_label, + dfsan_label endptr_label, + dfsan_label base_label, + dfsan_label *ret_label) { + return __dfsw_strtoll(nptr, endptr, base, nptr_label, endptr_label, + base_label, ret_label); +} + +SANITIZER_INTERFACE_ATTRIBUTE +unsigned long __dfsw___isoc23_strtoul(const char *nptr, char **endptr, int base, + dfsan_label nptr_label, + dfsan_label endptr_label, + dfsan_label base_label, + dfsan_label *ret_label) { + return __dfsw_strtoul(nptr, endptr, base, nptr_label, endptr_label, + base_label, ret_label); +} + +SANITIZER_INTERFACE_ATTRIBUTE +unsigned long long __dfsw___isoc23_strtoull(const char *nptr, char **endptr, + int base, dfsan_label nptr_label, + dfsan_label endptr_label, + dfsan_label base_label, + dfsan_label *ret_label) { + return __dfsw_strtoull(nptr, endptr, base, nptr_label, endptr_label, + base_label, ret_label); +} + SANITIZER_INTERFACE_ATTRIBUTE time_t __dfsw_time(time_t *t, dfsan_label t_label, dfsan_label *ret_label) { time_t ret = time(t); @@ -2432,27 +2476,20 @@ __dfsw_socketpair(int domain, int type, int protocol, int sv[2], return ret; } -// Type of the trampoline function passed to the custom version of -// dfsan_set_write_callback. -typedef void (*write_trampoline_t)( - void *callback, - int fd, const void *buf, ssize_t count, - dfsan_label fd_label, dfsan_label buf_label, dfsan_label count_label); +// Type of the write callback registered via dfsan_set_write_callback. +typedef void (*write_callback_t)(int fd, const void *buf, ssize_t count); -// Calls to dfsan_set_write_callback() set the values in this struct. -// Calls to the custom version of write() read (and invoke) them. +// Calls to dfsan_set_write_callback() set the value in this struct. +// Calls to the custom version of write() read (and invoke) it. static struct { - write_trampoline_t write_callback_trampoline = nullptr; - void *write_callback = nullptr; + write_callback_t write_callback = nullptr; } write_callback_info; SANITIZER_INTERFACE_ATTRIBUTE void __dfsw_dfsan_set_write_callback( - write_trampoline_t write_callback_trampoline, - void *write_callback, + write_callback_t write_callback, dfsan_label write_callback_label, dfsan_label *ret_label) { - write_callback_info.write_callback_trampoline = write_callback_trampoline; write_callback_info.write_callback = write_callback; *ret_label = 0; } @@ -2462,10 +2499,11 @@ __dfsw_write(int fd, const void *buf, size_t count, dfsan_label fd_label, dfsan_label buf_label, dfsan_label count_label, dfsan_label *ret_label) { if (write_callback_info.write_callback) { - write_callback_info.write_callback_trampoline( - write_callback_info.write_callback, - fd, buf, count, - fd_label, buf_label, count_label); + // Trampolines were removed with opaque pointers (LLVM 15+). The callback + // is now invoked directly; clear the args TLS so it sees zero-labelled + // arguments (label forwarding to this callback is not currently modelled). + dfsan_clear_thread_local_state(); + write_callback_info.write_callback(fd, buf, count); } *ret_label = 0; @@ -2489,7 +2527,7 @@ typedef int dfsan_label_va; struct Formatter { Formatter(char *str_, const char *fmt_, size_t size_) : str(str_), str_off(0), size(size_), fmt_start(fmt_), fmt_cur(fmt_), - width(-1) {} + width(-1), num_scanned(0), skip(false) {} int format() { char *tmp_fmt = build_format_string(); @@ -2514,15 +2552,46 @@ struct Formatter { return retval; } - char *build_format_string() { + // When with_n is true, append "%n" to the single-directive format so a real + // sscanf reports how many input characters this directive consumed. + char *build_format_string(bool with_n = false) { size_t fmt_size = fmt_cur - fmt_start + 1; - char *new_fmt = (char *)malloc(fmt_size + 1); + size_t n_size = with_n ? 2 : 0; // "%n" + char *new_fmt = (char *)malloc(fmt_size + n_size + 1); assert(new_fmt); internal_memcpy(new_fmt, fmt_start, fmt_size); - new_fmt[fmt_size] = '\0'; + if (with_n) { + new_fmt[fmt_size] = '%'; + new_fmt[fmt_size + 1] = 'n'; + } + new_fmt[fmt_size + n_size] = '\0'; return new_fmt; } + // Scan a suppressed/literal directive; returns the number of input characters + // consumed (via the appended %n). + int scan() { + char *tmp_fmt = build_format_string(true); + int read_count = 0; + int retval = sscanf(str + str_off, tmp_fmt, &read_count); + if (retval > 0) + num_scanned += retval; + free(tmp_fmt); + return read_count; + } + + // Scan one directive into 'arg'; returns the number of input characters + // consumed (via the appended %n). + template int scan(T arg) { + char *tmp_fmt = build_format_string(true); + int read_count = 0; + int retval = sscanf(str + str_off, tmp_fmt, arg, &read_count); + if (retval > 0) + num_scanned += retval; + free(tmp_fmt); + return read_count; + } + char *str_cur() { return str + str_off; } size_t num_written_bytes(int retval) { @@ -2551,6 +2620,8 @@ struct Formatter { const char *fmt_start; const char *fmt_cur; int width; + int num_scanned; // number of items assigned so far (sscanf return value) + bool skip; // current directive has assignment-suppression (%*) }; // Formats the input and propagates the input labels to the output. The output @@ -2712,6 +2783,229 @@ static int format_buffer(char *str, size_t size, const char *fmt, return formatter.str_off; } +// Scans 'str' according to 'format', propagating input taint to the scanned +// output arguments. Mirrors format_buffer / upstream DFSan's scan_buffer, but +// applies SymSan symbolic ops at the two label points: numeric conversions +// attach a string-to-int (fatoi) label so the solver can drive the raw input +// digits, and %s/%c copy the matched input bytes' labels to the destination. +// Per-directive consumed length is measured by an appended %n (Formatter::scan). +static int scan_buffer(char *str, size_t size, const char *fmt, + dfsan_label *va_labels, dfsan_label *ret_label, + va_list ap) { + Formatter formatter(str, fmt, size); + + while (*formatter.fmt_cur) { + formatter.fmt_start = formatter.fmt_cur; + formatter.width = -1; + formatter.skip = false; + int read_count = 0; + void *dst_ptr = 0; + size_t write_size = 0; + int base = 10; + + if (*formatter.fmt_cur != '%') { + // Ordinary characters up to the next '%'. + for (; *(formatter.fmt_cur + 1) && *(formatter.fmt_cur + 1) != '%'; + ++formatter.fmt_cur) {} + read_count = formatter.scan(); + dfsan_set_label(0, formatter.str_cur(), + formatter.num_written_bytes(read_count)); + } else { + bool end_fmt = false; + for (; *formatter.fmt_cur && !end_fmt; ) { + switch (*++formatter.fmt_cur) { + case 'd': + case 'i': + case 'u': + case 'o': + case 'x': + case 'X': + // base for the fatoi op; the solver supports only 2/8/10/16, and %i + // must NOT use C's auto base 0. + switch (*formatter.fmt_cur) { + case 'x': + case 'X': base = 16; break; + case 'o': base = 8; break; + default: base = 10; break; // d, i, u + } + if (formatter.skip) { + read_count = formatter.scan(); + } else { + switch (*(formatter.fmt_cur - 1)) { + case 'h': + // Also covers 'hh' (arg is promoted to int). + dst_ptr = va_arg(ap, int *); + read_count = formatter.scan((int *)dst_ptr); + write_size = sizeof(int); + break; + case 'l': + if (formatter.fmt_cur - formatter.fmt_start >= 2 && + *(formatter.fmt_cur - 2) == 'l') { + dst_ptr = va_arg(ap, long long int *); + read_count = formatter.scan((long long int *)dst_ptr); + write_size = sizeof(long long int); + } else { + dst_ptr = va_arg(ap, long int *); + read_count = formatter.scan((long int *)dst_ptr); + write_size = sizeof(long int); + } + break; + case 'q': + dst_ptr = va_arg(ap, long long int *); + read_count = formatter.scan((long long int *)dst_ptr); + write_size = sizeof(long long int); + break; + case 'j': + dst_ptr = va_arg(ap, intmax_t *); + read_count = formatter.scan((intmax_t *)dst_ptr); + write_size = sizeof(intmax_t); + break; + case 'z': + case 't': + dst_ptr = va_arg(ap, size_t *); + read_count = formatter.scan((size_t *)dst_ptr); + write_size = sizeof(size_t); + break; + default: + dst_ptr = va_arg(ap, int *); + read_count = formatter.scan((int *)dst_ptr); + write_size = sizeof(int); + } + // Attach a string-to-int (fatoi) label linking the parsed integer + // to the consumed input digits, so a constraint on the value is + // solved back into input bytes (same op as atoi/strtol). Pass + // add_null=false: this is a field inside a larger input, so the + // solver must not write a NUL after the digits (it would clobber + // the following separator/field). + dfsan_label l = taint_strtol( + formatter.str_cur(), formatter.num_written_bytes(read_count), + write_size, base, /*add_null=*/false); + dfsan_set_label(l, dst_ptr, write_size); + } + end_fmt = true; + break; + + case 'a': + case 'A': + case 'e': + case 'E': + case 'f': + case 'F': + case 'g': + case 'G': + if (formatter.skip) { + read_count = formatter.scan(); + } else { + if (*(formatter.fmt_cur - 1) == 'L') { + dst_ptr = va_arg(ap, long double *); + read_count = formatter.scan((long double *)dst_ptr); + write_size = sizeof(long double); + } else if (*(formatter.fmt_cur - 1) == 'l') { + dst_ptr = va_arg(ap, double *); + read_count = formatter.scan((double *)dst_ptr); + write_size = sizeof(double); + } else { + dst_ptr = va_arg(ap, float *); + read_count = formatter.scan((float *)dst_ptr); + write_size = sizeof(float); + } + // No symbolic float model yet (strtod is a stub); clear shadow. + dfsan_set_label(0, dst_ptr, write_size); + } + end_fmt = true; + break; + + case 'c': + if (formatter.skip) { + read_count = formatter.scan(); + } else { + dst_ptr = va_arg(ap, char *); + read_count = formatter.scan((char *)dst_ptr); + write_size = sizeof(char); + // Copy the matched input byte's label to the output char. + dfsan_label l = dfsan_read_label( + formatter.str_cur(), formatter.num_written_bytes(read_count)); + dfsan_set_label(l, dst_ptr, write_size); + } + end_fmt = true; + break; + + case 's': { + if (formatter.skip) { + read_count = formatter.scan(); + } else { + dst_ptr = va_arg(ap, char *); + read_count = formatter.scan((char *)dst_ptr); + if (1 == read_count) { + // one string matched: use its actual length + read_count = internal_strlen((char *)dst_ptr); + } + va_labels++; + // Copy the matched input bytes' labels to the output buffer (the + // printf-%s idiom, in reverse): downstream strcmp/etc. is solvable. + internal_memcpy(shadow_for(dst_ptr), + shadow_for(formatter.str_cur()), + sizeof(dfsan_label) * + formatter.num_written_bytes(read_count)); + } + end_fmt = true; + break; + } + + case 'p': + if (formatter.skip) { + read_count = formatter.scan(); + } else { + dst_ptr = va_arg(ap, void *); + read_count = formatter.scan((int *)dst_ptr); + write_size = sizeof(int); + dfsan_set_label(0, dst_ptr, write_size); + } + end_fmt = true; + break; + + case 'n': { + if (!formatter.skip) { + int *ptr = va_arg(ap, int *); + *ptr = (int)formatter.str_off; + va_labels++; + dfsan_set_label(0, ptr, sizeof(*ptr)); + } + end_fmt = true; + break; + } + + case '%': + read_count = formatter.scan(); + end_fmt = true; + break; + + case '*': + formatter.skip = true; + break; + + default: + break; + } + } + } + + if (read_count < 0) { + // Matching failure / EOF. + return read_count; + } + + formatter.fmt_cur++; + formatter.str_off += read_count; + } + + (void)va_labels; + *ret_label = 0; + + // Number of items scanned in total. + return formatter.num_scanned; +} + extern "C" { SANITIZER_INTERFACE_ATTRIBUTE int __dfsw_sprintf(char *str, const char *format, dfsan_label str_label, @@ -2719,7 +3013,9 @@ int __dfsw_sprintf(char *str, const char *format, dfsan_label str_label, dfsan_label *ret_label, ...) { va_list ap; va_start(ap, ret_label); - int ret = format_buffer(str, ~0ul, format, va_labels, ret_label, ap); + // Do not use a ~0 size: on glibc >= 2.37 / musl, snprintf computes `str + n` + // which wraps for an unbounded size and drops the last char (glibc PR30441). + int ret = format_buffer(str, INT32_MAX, format, va_labels, ret_label, ap); va_end(ap); *ret_label = 0; return ret; @@ -2738,6 +3034,44 @@ int __dfsw_snprintf(char *str, size_t size, const char *format, return ret; } +SANITIZER_INTERFACE_ATTRIBUTE +int __dfsw_sscanf(char *str, const char *format, dfsan_label str_label, + dfsan_label format_label, dfsan_label *va_labels, + dfsan_label *ret_label, ...) { + va_list ap; + va_start(ap, ret_label); + int ret = scan_buffer(str, INT32_MAX, format, va_labels, ret_label, ap); + va_end(ap); + *ret_label = 0; + return ret; +} + +// glibc redirects sscanf -> __isoc99_sscanf (C99), and -> __isoc23_sscanf on +// glibc 2.38+, so these aliases are what real code actually calls. +SANITIZER_INTERFACE_ATTRIBUTE +int __dfsw___isoc99_sscanf(char *str, const char *format, dfsan_label str_label, + dfsan_label format_label, dfsan_label *va_labels, + dfsan_label *ret_label, ...) { + va_list ap; + va_start(ap, ret_label); + int ret = scan_buffer(str, INT32_MAX, format, va_labels, ret_label, ap); + va_end(ap); + *ret_label = 0; + return ret; +} + +SANITIZER_INTERFACE_ATTRIBUTE +int __dfsw___isoc23_sscanf(char *str, const char *format, dfsan_label str_label, + dfsan_label format_label, dfsan_label *va_labels, + dfsan_label *ret_label, ...) { + va_list ap; + va_start(ap, ret_label); + int ret = scan_buffer(str, INT32_MAX, format, va_labels, ret_label, ap); + va_end(ap); + *ret_label = 0; + return ret; +} + // Default empty implementations (weak). Users should redefine them. SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_pc_guard, u32 *) {} SANITIZER_INTERFACE_WEAK_DEF(void, __sanitizer_cov_trace_pc_guard_init, u32 *, diff --git a/runtime/dfsan/done_abilist.txt b/runtime/dfsan/done_abilist.txt index a588c823..649bc67e 100644 --- a/runtime/dfsan/done_abilist.txt +++ b/runtime/dfsan/done_abilist.txt @@ -357,6 +357,11 @@ fun:strtol=custom fun:strtoll=custom fun:strtoul=custom fun:strtoull=custom +# glibc 2.38 (C23) redirects the strto* family to these at compile time +fun:__isoc23_strtol=custom +fun:__isoc23_strtoll=custom +fun:__isoc23_strtoul=custom +fun:__isoc23_strtoull=custom fun:atoi=custom fun:atol=custom fun:atoll=custom @@ -471,6 +476,11 @@ fun:gettimeofday=custom fun:sprintf=custom fun:snprintf=custom +# scanf-like (glibc redirects sscanf -> __isoc99_sscanf / __isoc23_sscanf) +fun:sscanf=custom +fun:__isoc99_sscanf=custom +fun:__isoc23_sscanf=custom + # TODO: custom fun:asprintf=discard fun:qsort=discard diff --git a/runtime/sanitizer_common/sanitizer_common.cpp b/runtime/sanitizer_common/sanitizer_common.cpp index e9379b7b..9a0549a7 100644 --- a/runtime/sanitizer_common/sanitizer_common.cpp +++ b/runtime/sanitizer_common/sanitizer_common.cpp @@ -53,6 +53,26 @@ void NORETURN ReportMmapFailureAndDie(uptr size, const char *mem_type, UNREACHABLE("unable to mmap"); } +void NORETURN ReportMunmapFailureAndDie(void *addr, uptr size, error_t err, + bool raw_report) { + static int recursion_count; + if (raw_report || recursion_count) { + // If raw report is requested or we went into recursion just die. The + // Report() and CHECK calls below may call mmap recursively and fail. + RawWrite("ERROR: Failed to mmap\n"); + Die(); + } + recursion_count++; + Report( + "ERROR: %s failed to deallocate 0x%zx (%zd) bytes at address %p (error " + "code: %d)\n", + SanitizerToolName, size, size, addr, err); +#if !SANITIZER_GO + DumpProcessMap(); +#endif + UNREACHABLE("unable to unmmap"); +} + typedef bool UptrComparisonFunction(const uptr &a, const uptr &b); typedef bool U32ComparisonFunction(const u32 &a, const u32 &b); diff --git a/runtime/sanitizer_common/sanitizer_common.h b/runtime/sanitizer_common/sanitizer_common.h index 3302590c..94f4088a 100644 --- a/runtime/sanitizer_common/sanitizer_common.h +++ b/runtime/sanitizer_common/sanitizer_common.h @@ -310,6 +310,8 @@ CheckFailed(const char *file, int line, const char *cond, u64 v1, u64 v2); void NORETURN ReportMmapFailureAndDie(uptr size, const char *mem_type, const char *mmap_type, error_t err, bool raw_report = false); +void NORETURN ReportMunmapFailureAndDie(void *ptr, uptr size, error_t err, + bool raw_report = false); // Specific tools may override behavior of "Die" function to do tool-specific // job. diff --git a/runtime/sanitizer_common/sanitizer_platform.h b/runtime/sanitizer_common/sanitizer_platform.h index 8de765cf..4925bef4 100644 --- a/runtime/sanitizer_common/sanitizer_platform.h +++ b/runtime/sanitizer_common/sanitizer_platform.h @@ -294,9 +294,12 @@ // The AArch64 and RISC-V linux ports use the canonical syscall set as // mandated by the upstream linux community for all new ports. Other ports // may still use legacy syscalls. +// LLVM 18 (34b676eb60ca) uses the canonical syscalls on all Linux; mirror that +// here by enabling them for every SANITIZER_LINUX target (incl. x86_64) so we +// issue openat/newfstatat/dup3/... instead of the legacy open/stat/dup2 (more +// robust under seccomp filters that only allow the canonical set). #ifndef SANITIZER_USES_CANONICAL_LINUX_SYSCALLS -# if (defined(__aarch64__) || defined(__riscv) || defined(__hexagon__)) && \ - SANITIZER_LINUX +# if SANITIZER_LINUX # define SANITIZER_USES_CANONICAL_LINUX_SYSCALLS 1 # else # define SANITIZER_USES_CANONICAL_LINUX_SYSCALLS 0 diff --git a/runtime/sanitizer_common/sanitizer_posix.cpp b/runtime/sanitizer_common/sanitizer_posix.cpp index f8457a6a..afdeaf15 100644 --- a/runtime/sanitizer_common/sanitizer_posix.cpp +++ b/runtime/sanitizer_common/sanitizer_posix.cpp @@ -55,11 +55,9 @@ void *MmapOrDie(uptr size, const char *mem_type, bool raw_report) { void UnmapOrDie(void *addr, uptr size) { if (!addr || !size) return; uptr res = internal_munmap(addr, size); - if (UNLIKELY(internal_iserror(res))) { - Report("ERROR: %s failed to deallocate 0x%zx (%zd) bytes at address %p\n", - SanitizerToolName, size, size, addr); - CHECK("unable to unmap" && 0); - } + int reserrno; + if (UNLIKELY(internal_iserror(res, &reserrno))) + ReportMunmapFailureAndDie(addr, size, reserrno); DecreaseTotalMmap(size); } diff --git a/runtime/sanitizer_common/sanitizer_symbolizer_internal.h b/runtime/sanitizer_common/sanitizer_symbolizer_internal.h index df122ed3..29a08386 100644 --- a/runtime/sanitizer_common/sanitizer_symbolizer_internal.h +++ b/runtime/sanitizer_common/sanitizer_symbolizer_internal.h @@ -90,9 +90,10 @@ class SymbolizerProcess { // Customizable by subclasses. virtual bool StartSymbolizerSubprocess(); - virtual bool ReadFromSymbolizer(char *buffer, uptr max_length); + virtual bool ReadFromSymbolizer(); // Return the environment to run the symbolizer in. virtual char **GetEnvP() { return GetEnviron(); } + InternalMmapVector &GetBuff() { return buffer_; } private: virtual bool ReachedEndOfOutput(const char *buffer, uptr length) const { @@ -113,8 +114,7 @@ class SymbolizerProcess { fd_t input_fd_; fd_t output_fd_; - static const uptr kBufferSize = 16 * 1024; - char buffer_[kBufferSize]; + InternalMmapVector buffer_; static const uptr kMaxTimesRestarted = 5; static const int kSymbolizerStartupTimeMillis = 10; diff --git a/runtime/sanitizer_common/sanitizer_symbolizer_libcdep.cpp b/runtime/sanitizer_common/sanitizer_symbolizer_libcdep.cpp index 8bbd4af0..3fead87e 100644 --- a/runtime/sanitizer_common/sanitizer_symbolizer_libcdep.cpp +++ b/runtime/sanitizer_common/sanitizer_symbolizer_libcdep.cpp @@ -117,7 +117,7 @@ bool Symbolizer::SymbolizeData(uptr addr, DataInfo *info) { return true; } } - return true; + return false; } bool Symbolizer::SymbolizeFrame(uptr addr, FrameInfo *info) { @@ -133,7 +133,7 @@ bool Symbolizer::SymbolizeFrame(uptr addr, FrameInfo *info) { return true; } } - return true; + return false; } bool Symbolizer::GetModuleNameAndOffsetForPC(uptr pc, const char **module_name, @@ -500,9 +500,9 @@ const char *SymbolizerProcess::SendCommandImpl(const char *command) { return nullptr; if (!WriteToSymbolizer(command, internal_strlen(command))) return nullptr; - if (!ReadFromSymbolizer(buffer_, kBufferSize)) - return nullptr; - return buffer_; + if (!ReadFromSymbolizer()) + return nullptr; + return buffer_.data(); } bool SymbolizerProcess::Restart() { @@ -513,31 +513,33 @@ bool SymbolizerProcess::Restart() { return StartSymbolizerSubprocess(); } -bool SymbolizerProcess::ReadFromSymbolizer(char *buffer, uptr max_length) { - if (max_length == 0) - return true; - uptr read_len = 0; - while (true) { +bool SymbolizerProcess::ReadFromSymbolizer() { + buffer_.clear(); + constexpr uptr max_length = 1024; + bool ret = true; + do { uptr just_read = 0; - bool success = ReadFromFile(input_fd_, buffer + read_len, - max_length - read_len - 1, &just_read); + uptr size_before = buffer_.size(); + buffer_.resize(size_before + max_length); + buffer_.resize(buffer_.capacity()); + bool read_ok = ReadFromFile(input_fd_, &buffer_[size_before], + buffer_.size() - size_before, &just_read); + + if (!read_ok) + just_read = 0; + + buffer_.resize(size_before + just_read); + // We can't read 0 bytes, as we don't expect external symbolizer to close // its stdout. - if (!success || just_read == 0) { + if (just_read == 0) { Report("WARNING: Can't read from symbolizer at fd %d\n", input_fd_); - return false; - } - read_len += just_read; - if (ReachedEndOfOutput(buffer, read_len)) - break; - if (read_len + 1 == max_length) { - Report("WARNING: Symbolizer buffer too small\n"); - read_len = 0; + ret = false; break; } - } - buffer[read_len] = '\0'; - return true; + } while (!ReachedEndOfOutput(buffer_.data(), buffer_.size())); + buffer_.push_back('\0'); + return ret; } bool SymbolizerProcess::WriteToSymbolizer(const char *buffer, uptr length) { diff --git a/runtime/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp b/runtime/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp index 5f6e4cc3..7b6323ab 100644 --- a/runtime/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp +++ b/runtime/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cpp @@ -225,19 +225,16 @@ class Addr2LineProcess final : public SymbolizerProcess { bool ReachedEndOfOutput(const char *buffer, uptr length) const override; - bool ReadFromSymbolizer(char *buffer, uptr max_length) override { - if (!SymbolizerProcess::ReadFromSymbolizer(buffer, max_length)) + bool ReadFromSymbolizer() override { + if (!SymbolizerProcess::ReadFromSymbolizer()) return false; - // The returned buffer is empty when output is valid, but exceeds - // max_length. - if (*buffer == '\0') - return true; + auto &buff = GetBuff(); // We should cut out output_terminator_ at the end of given buffer, // appended by addr2line to mark the end of its meaningful output. // We cannot scan buffer from it's beginning, because it is legal for it // to start with output_terminator_ in case given offset is invalid. So, // scanning from second character. - char *garbage = internal_strstr(buffer + 1, output_terminator_); + char *garbage = internal_strstr(buff.data() + 1, output_terminator_); // This should never be NULL since buffer must end up with // output_terminator_. CHECK(garbage); diff --git a/solvers/jigsaw/jit.cc b/solvers/jigsaw/jit.cc index 94ae868b..106e7f9b 100644 --- a/solvers/jigsaw/jit.cc +++ b/solvers/jigsaw/jit.cc @@ -431,6 +431,12 @@ int rgd::addFunction(const AstNode* node, test_fn_type rgd::performJit(uint64_t id) { std::string funcName = "rgdjit_f" + std::to_string(id); auto ExprSymbol = JIT->lookup(funcName).get(); + // LLVM 17 changed getAddress() to return an ExecutorAddr wrapper instead of + // a raw uint64_t JITTargetAddress. +#if LLVM_VERSION_MAJOR >= 17 + auto func = (test_fn_type)ExprSymbol.getAddress().getValue(); +#else auto func = (test_fn_type)ExprSymbol.getAddress(); +#endif return func; } diff --git a/solvers/jigsaw/rgdJit.h b/solvers/jigsaw/rgdJit.h index df916709..abc48eb6 100644 --- a/solvers/jigsaw/rgdJit.h +++ b/solvers/jigsaw/rgdJit.h @@ -95,7 +95,10 @@ class GradJit { llvm::orc::ThreadSafeModule(std::move(M), std::move(ctx)))); } - llvm::Expected lookup(llvm::StringRef Name) { + // LLVM 17 changed ExecutionSession::lookup to return + // Expected instead of Expected; let + // the return type be deduced so this compiles across versions. + auto lookup(llvm::StringRef Name) { return ES->lookup({&MainJD}, Mangle(Name.str())); } diff --git a/solvers/jit-solver.cpp b/solvers/jit-solver.cpp index bed99a6a..54b7195a 100644 --- a/solvers/jit-solver.cpp +++ b/solvers/jit-solver.cpp @@ -3,6 +3,7 @@ #include "solver.h" #include "ast.h" +#include "dfsan/dfsan.h" // FATOI_NO_NULL / FATOI_BASE_MASK #include "jigsaw/rgdJit.h" #include "jigsaw/jit.h" #include "wheels/lockfreehash/lprobe/hash_table.h" @@ -145,7 +146,9 @@ JITSolver::solve(std::shared_ptr task, assert(itr != task->solution.end()); val |= itr->second << (8 * (i - 1)); } - uint32_t base = std::get<1>(info); + uint32_t raw_base = std::get<1>(info); + bool skip_null = (raw_base & FATOI_NO_NULL) != 0; + uint32_t base = raw_base & FATOI_BASE_MASK; uint32_t orig_len = std::get<2>(info); DEBUGF("generate_input atoi offset:%d => %lu, base = %d, original len = %d\n", offset, val, base, orig_len); @@ -158,7 +161,20 @@ JITSolver::solve(std::shared_ptr task, default: WARNF("unsupported base %d\n", base); } if (format) { - snprintf((char*)out_buf + offset, in_size - offset, format, val); + if (skip_null) { + // Number embedded in a larger input (e.g. an sscanf field): write + // just the digits so snprintf's NUL doesn't clobber the following + // separator/field. + char tmp[64]; + int m = snprintf(tmp, sizeof(tmp), format, val); + if (m > 0) { + if ((size_t)offset + (size_t)m > in_size) + m = (int)(in_size - offset); + memcpy(out_buf + offset, tmp, m); + } + } else { + snprintf((char*)out_buf + offset, in_size - offset, format, val); + } } } } diff --git a/solvers/z3-ts.cpp b/solvers/z3-ts.cpp index 4aae3b6e..50862b98 100644 --- a/solvers/z3-ts.cpp +++ b/solvers/z3-ts.cpp @@ -147,7 +147,7 @@ void Z3AstParser::dump_value_cache(dfsan_label label) { Z3AstParser::Z3AstParser(void *base, size_t size, z3::context &context) : ASTParser(base, size), context_(context) { input_name_format = "input-%u-%u"; - atoi_name_format = "atoi-%u-%u-%d-%lu"; // input, offset, base, original_len + atoi_name_format = "atoi-%u-%u-%d-%lu-%d"; // input, offset, base, original_len, skip_null strlen_name_format = "strlen-%u-%u-%lu-%u"; // input, offset, original_len, null_from_input str_name_format = "str-%u-%u-%u"; // input, offset, length int_name_format = "int-%u-%u-%u"; // input, offset, bits @@ -588,15 +588,21 @@ z3::expr Z3AstParser::serialize(dfsan_label label, input_dep_set_t &deps) { // string to integer conversion assert(info->l1 == 0 && info->l2 >= CONST_OFFSET); dfsan_label_info *src = get_label_info(info->l2); - assert(src->op == __dfsan::Load); - uint32_t offset = get_label_info(src->l1)->op1.i; // legacy: offset in op1 - uint32_t input = get_label_info(src->l1)->op2.i; - int base = info->op1.i; + // The consumed digits are represented either as a Load label (len > 1; + // walk Load.l1 to the first input byte) or, when a single digit was + // consumed, as the raw input-byte label directly (op == 0). + assert(src->op == __dfsan::Load || src->op == 0); + dfsan_label_info *byte = + (src->op == __dfsan::Load) ? get_label_info(src->l1) : src; + uint32_t offset = byte->op1.i; // legacy: offset in op1 + uint32_t input = byte->op2.i; + int base = info->op1.i & FATOI_BASE_MASK; + int skip_null = (info->op1.i & FATOI_NO_NULL) ? 1 : 0; uint64_t orig_len = info->op2.i; // FIXME: dependencies? tsize_cache_.emplace_back(1); // XXX: hacky, avoid string theory - snprintf(name, sizeof(name), atoi_name_format, input, offset, base, orig_len); + snprintf(name, sizeof(name), atoi_name_format, input, offset, base, orig_len, skip_null); z3::symbol symbol = context_.str_symbol(name); z3::sort sort = context_.bv_sort(info->size); cache_expr(l, context_.constant(symbol, sort)); @@ -2590,9 +2596,10 @@ void Z3ParserSolver::generate_solution(z3::model &m, solution_t &solutions) { uint32_t offset; int base; uint64_t orig_len; + int skip_null = 0; char buf[64]; - int parsed = sscanf(name.str().c_str(), atoi_name_format, &input, &offset, &base, &orig_len); - if (parsed != 4) { + int parsed = sscanf(name.str().c_str(), atoi_name_format, &input, &offset, &base, &orig_len, &skip_null); + if (parsed != 5) { continue; } const char *format = NULL; @@ -2629,8 +2636,11 @@ void Z3ParserSolver::generate_solution(z3::model &m, solution_t &solutions) { solutions.emplace_back(input, offset + i, (uint8_t)buf[i]); } } - // Set null terminator at the new end - solutions.emplace_back(input, offset + new_len, (uint8_t)0); + // Set null terminator at the new end, unless this number is embedded in + // a larger input (e.g. an sscanf field), where a NUL would clobber the + // following separator/field. + if (!skip_null) + solutions.emplace_back(input, offset + new_len, (uint8_t)0); } else if (name.str().find("strlen") == 0) { uint32_t input; uint32_t offset; diff --git a/tests/sscanf_int.c b/tests/sscanf_int.c new file mode 100644 index 00000000..0a113ad7 --- /dev/null +++ b/tests/sscanf_int.c @@ -0,0 +1,51 @@ +// Test: sscanf numeric specifiers (%d %i %u %x %X %o) drive the solver via the +// fatoi (string-to-int) op, covering all supported bases (10/16/8). The %d +// field is single-digit, exercising the len==1 fatoi path. The other fields +// extend from one digit, exercising the "no NUL after an embedded field" +// solution path (a trailing NUL would clobber the space separators). +// RUN: rm -rf %t.out +// RUN: mkdir -p %t.out +// RUN: printf '0 0 0 0 0 0' > %t.bin +// RUN: clang -o %t.uninstrumented %s +// RUN: %t.uninstrumented %t.bin | FileCheck --check-prefix=CHECK-ORIG %s +// RUN: env KO_USE_FASTGEN=1 %ko-clang -o %t.fg %s +// RUN: env TAINT_OPTIONS="taint_file=%t.bin output_dir=%t.out" %fgtest %t.fg %t.bin +// RUN: %t.uninstrumented %t.out/id-0-0-0 | FileCheck --check-prefix=CHECK-GEN %s + +#include +#include + +int main(int argc, char **argv) { + if (argc < 2) { + fprintf(stderr, "Usage: %s \n", argv[0]); + return 1; + } + + char buf[128] = {0}; + FILE *f = fopen(argv[1], "rb"); + if (!f) { + perror("fopen"); + return 1; + } + size_t n = fread(buf, 1, sizeof(buf) - 1, f); + buf[n] = '\0'; + fclose(f); + + int d = 0, i = 0; + unsigned u = 0, x = 0, X = 0, o = 0; + sscanf(buf, "%d %i %u %x %X %o", &d, &i, &u, &x, &X, &o); + + // Bitwise & (no short-circuit) makes a single branch depending on all six + // parsed values, so the solver produces one input (id-0-0-0). Bases: + // d/i/u=10, x/X=16, o=8; d==7 is a single-digit (len==1) field. + int ok = (d == 7) & (i == 22) & (u == 33) & (x == 0x44) & (X == 0x5a) & + (o == 0666); + if (ok) { + // CHECK-GEN: SCANF-INT-OK + printf("SCANF-INT-OK\n"); + } else { + // CHECK-ORIG: SCANF-INT-NO + printf("SCANF-INT-NO\n"); + } + return 0; +} diff --git a/tests/sscanf_str.c b/tests/sscanf_str.c new file mode 100644 index 00000000..477acd46 --- /dev/null +++ b/tests/sscanf_str.c @@ -0,0 +1,42 @@ +// Test: sscanf %s copies the matched input bytes' labels to the output buffer, +// so a downstream strcmp on the scanned string is solvable. +// RUN: rm -rf %t.out +// RUN: mkdir -p %t.out +// RUN: printf 'aaaaa' > %t.bin +// RUN: clang -o %t.uninstrumented %s +// RUN: %t.uninstrumented %t.bin | FileCheck --check-prefix=CHECK-ORIG %s +// RUN: env KO_USE_FASTGEN=1 %ko-clang -o %t.fg %s +// RUN: env TAINT_OPTIONS="taint_file=%t.bin output_dir=%t.out" %fgtest %t.fg %t.bin +// RUN: %t.uninstrumented %t.out/id-0-0-0 | FileCheck --check-prefix=CHECK-GEN %s + +#include +#include +#include + +int main(int argc, char **argv) { + if (argc < 2) { + fprintf(stderr, "Usage: %s \n", argv[0]); + return 1; + } + + char buf[128] = {0}; + FILE *f = fopen(argv[1], "rb"); + if (!f) { + perror("fopen"); + return 1; + } + size_t n = fread(buf, 1, sizeof(buf) - 1, f); + buf[n] = '\0'; + fclose(f); + + char s[64] = {0}; + sscanf(buf, "%s", s); + if (strcmp(s, "hello") == 0) { + // CHECK-GEN: SCANF-STR-OK + printf("SCANF-STR-OK\n"); + } else { + // CHECK-ORIG: SCANF-STR-NO + printf("SCANF-STR-NO\n"); + } + return 0; +} diff --git a/tests/strsub.c b/tests/strsub.c index e9478b89..41097ceb 100644 --- a/tests/strsub.c +++ b/tests/strsub.c @@ -5,7 +5,7 @@ // RUN: %t.uninstrumented %t.bin | FileCheck --check-prefix=CHECK-ORIG %s // RUN: env KO_USE_FASTGEN=1 %ko-clang -o %t.fg %s // RUN: env TAINT_OPTIONS="taint_file=%t.bin output_dir=%t.out" %fgtest %t.fg %t.bin -// RUN: %t.uninstrumented %t.out/id-0-0-3 | FileCheck --check-prefix=CHECK-GEN %s +// RUN: %t.uninstrumented %t.out/id-0-0-1 | FileCheck --check-prefix=CHECK-GEN %s #include #include