microsoft / DirectXShaderCompiler

This repo hosts the source for the DirectX Shader Compiler which is based on LLVM/Clang.
Other
3.04k stars 677 forks source link

ClangTidy: clang-analyzer-core.uninitialized.Branch in third_party/dawn/third_party/dxc/tools/clang/tools/libclang/CIndex.cpp #5993

Open farzonl opened 10 months ago

farzonl commented 10 months ago

Description A preprocessor defintion to block out code not needed for dxc means that TU is expected to be set via clang_createTranslationUnit2's *out_TU = MakeCXTranslationUnit(CXXIdx, AU.release()); Even though it is not necessary for it to do so becauseCXTranslationUnit TU; is return by value via clang_createTranslationUnit.

https://github.com/microsoft/DirectXShaderCompiler/blob/ceff9b8043df3ac8d7d3ce71b409d83afcd7925b/tools/clang/tools/libclang/CIndex.cpp#L2944:2961

While this is a false positive it is the only case in chromium third party (https://chromium.googlesource.com/external/github.com/microsoft/DirectXShaderCompiler) that violates this rule.

An altenative:

CXTranslationUnit clang_createTranslationUnit(CXIndex CIdx,
                                              const char *ast_filename) {
  CXTranslationUnit TU;
#if 1 // HLSL Change Starts - no support for serialization
  enum CXErrorCode Result = CXError_Failure;
#else 
 enum CXErrorCode Result = 
      clang_createTranslationUnit2(CIdx, ast_filename, &TU);
#endif // HLSL Change Ends - no support for serialization
  (void)Result;
  assert((TU && Result == CXError_Success) ||
         (!TU && Result != CXError_Success));
  return TU;
}

This way you avoid calling clang_createTranslationUnit2 entirely.

llvm-beanz commented 10 months ago

I'd probably change this to be something more like:

CXTranslationUnit clang_createTranslationUnit(CXIndex CIdx,
                                              const char *ast_filename) {
#if 1
  return CXTranslationUnit();
#else
  CXTranslationUnit TU;
  enum CXErrorCode Result =
      clang_createTranslationUnit2(CIdx, ast_filename, &TU);
  (void)Result;
  assert((TU && Result == CXError_Success) ||
         (!TU && Result != CXError_Success));
  return TU;
#endif
}

Everything else in the function is effectively unused.

@farzonl feel like making a PR?