[llvm-commits] [llvm] r171413 - /llvm/trunk/include/llvm/ADT/Optional.h
Thomas Schwinge
thomas at codesourcery.com
Fri Jan 4 06:06:18 PST 2013
Hi!
On Wed, 02 Jan 2013 21:19:10 -0000, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> Author: akirtzidis
> Date: Wed Jan 2 15:19:08 2013
> New Revision: 171413
>
> URL: http://llvm.org/viewvc/llvm-project?rev=171413&view=rev
> Log:
> Use a bool instead of a bitfield in llvm/ADT/Optional.
>
> Fixes Valgrind failures and removes bitwise operations that don't provide any benefit.
> Valgrind failures reported by NAKAMURA Takumi.
Just for the record, as I also observed this issue: now that I'm using
your patch, I no longer need to use my own workaround, which was the
following:
commit a90b3a49711581ab2f99ae5dd7da7c145a3fd92d
Author: Thomas Schwinge <thomas at codesourcery.com>
Date: Fri Jan 4 12:17:34 2013 +0100
llvm::LockFileManager::getState: Work around bug causing uninitialised values being used.
For example:
$ make -k VERBOSE=1 LIT_ARGS='-v --vg --shuffle --filter=Modules/' check-all
[...]
FAIL: Clang :: Modules/decldef.mm (1 of 39)
******************** TEST 'Clang :: Modules/decldef.mm' FAILED ********************
Script:
--
rm -rf [...]/tools/clang/test/Modules/Output/decldef.mm.tmp
[...]/Release+Debug+Asserts/bin/clang -cc1 -internal-isystem [...]/Release+Debug+Asserts/bin/../lib/clang/3.3/include -fmodules -I [...]/test/Modules/Inputs -fmodule-cache-path [...]/tools/clang/test/Modules/Output/decldef.mm.tmp [...]/test/Modules/decldef.mm -verify
--
Exit Code: 123
Command Output (stderr):
--
==15885== Conditional jump or move depends on uninitialised value(s)
==15885== at 0x5299C9: compileModule(clang::CompilerInstance&, clang::SourceLocation, clang::Module*, llvm::StringRef) (CompilerInstance.cpp:769)
==15885== by 0x596ACE: clang::CompilerInstance::loadModule(clang::SourceLocation, llvm::ArrayRef<std::pair<clang::IdentifierInfo*, clang::SourceLocation> >, clang::Module::NameVisibilityKind, bool) (CompilerInstance.cpp:984)
==15885== by 0x12944DF: clang::Preprocessor::LexAfterModuleImport(clang::Token&) (Preprocessor.cpp:686)
==15885== by 0x897B64: clang::Parser::ConsumeToken() (Preprocessor.h:693)
==15885== by 0x893378: clang::Parser::ParseModuleImport(clang::SourceLocation) (Parser.cpp:1871)
==15885== by 0x8E419B: clang::Parser::ParseObjCAtDirectives() (ParseObjc.cpp:70)
==15885== by 0x8948F1: clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) (Parser.cpp:674)
==15885== by 0x894ED7: clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&) (Parser.cpp:560)
==15885== by 0x88CA84: clang::ParseAST(clang::Sema&, bool, bool) (ParseAST.cpp:123)
==15885== by 0x5AEBF8: clang::FrontendAction::Execute() (FrontendAction.cpp:381)
==15885== by 0x593A67: clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (CompilerInstance.cpp:703)
==15885== by 0x57B225: clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (ExecuteCompilerInvocation.cpp:234)
==15885==
==15885== Conditional jump or move depends on uninitialised value(s)
==15885== at 0x5299CE: compileModule(clang::CompilerInstance&, clang::SourceLocation, clang::Module*, llvm::StringRef) (CompilerInstance.cpp:769)
==15885== by 0x596ACE: clang::CompilerInstance::loadModule(clang::SourceLocation, llvm::ArrayRef<std::pair<clang::IdentifierInfo*, clang::SourceLocation> >, clang::Module::NameVisibilityKind, bool) (CompilerInstance.cpp:984)
==15885== by 0x12944DF: clang::Preprocessor::LexAfterModuleImport(clang::Token&) (Preprocessor.cpp:686)
==15885== by 0x897B64: clang::Parser::ConsumeToken() (Preprocessor.h:693)
==15885== by 0x893378: clang::Parser::ParseModuleImport(clang::SourceLocation) (Parser.cpp:1871)
==15885== by 0x8E419B: clang::Parser::ParseObjCAtDirectives() (ParseObjc.cpp:70)
==15885== by 0x8948F1: clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) (Parser.cpp:674)
==15885== by 0x894ED7: clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&) (Parser.cpp:560)
==15885== by 0x88CA84: clang::ParseAST(clang::Sema&, bool, bool) (ParseAST.cpp:123)
==15885== by 0x5AEBF8: clang::FrontendAction::Execute() (FrontendAction.cpp:381)
==15885== by 0x593A67: clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (CompilerInstance.cpp:703)
==15885== by 0x57B225: clang::ExecuteCompilerInvocation(clang::CompilerInstance*) (ExecuteCompilerInvocation.cpp:234)
==15885==
error: no expected directives found: consider use of 'expected-no-diagnostics'
1 error generated.
==15885== Conditional jump or move depends on uninitialised value(s)
==15885== at 0x5D3AB22: llvm::LockFileManager::~LockFileManager() (LockFileManager.cpp:156)
==15885== by 0x52AB11: compileModule(clang::CompilerInstance&, clang::SourceLocation, clang::Module*, llvm::StringRef) (CompilerInstance.cpp:889)
==15885== by 0x596ACE: clang::CompilerInstance::loadModule(clang::SourceLocation, llvm::ArrayRef<std::pair<clang::IdentifierInfo*, clang::SourceLocation> >, clang::Module::NameVisibilityKind, bool) (CompilerInstance.cpp:984)
==15885== by 0x12944DF: clang::Preprocessor::LexAfterModuleImport(clang::Token&) (Preprocessor.cpp:686)
==15885== by 0x897B64: clang::Parser::ConsumeToken() (Preprocessor.h:693)
==15885== by 0x893378: clang::Parser::ParseModuleImport(clang::SourceLocation) (Parser.cpp:1871)
==15885== by 0x8E419B: clang::Parser::ParseObjCAtDirectives() (ParseObjc.cpp:70)
==15885== by 0x8948F1: clang::Parser::ParseExternalDeclaration(clang::Parser::ParsedAttributesWithRange&, clang::ParsingDeclSpec*) (Parser.cpp:674)
==15885== by 0x894ED7: clang::Parser::ParseTopLevelDecl(clang::OpaquePtr<clang::DeclGroupRef>&) (Parser.cpp:560)
==15885== by 0x88CA84: clang::ParseAST(clang::Sema&, bool, bool) (ParseAST.cpp:123)
==15885== by 0x5AEBF8: clang::FrontendAction::Execute() (FrontendAction.cpp:381)
==15885== by 0x593A67: clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) (CompilerInstance.cpp:703)
==15885==
diff --git lib/Support/LockFileManager.cpp lib/Support/LockFileManager.cpp
index 075d8a5..a42b1cc 100644
--- lib/Support/LockFileManager.cpp
+++ lib/Support/LockFileManager.cpp
@@ -142,6 +142,7 @@ LockFileManager::LockFileManager(StringRef FileName)
Error = EC;
}
+__attribute__((optimize(1)))
LockFileManager::LockFileState LockFileManager::getState() const {
if (Owner)
return LFS_Shared;
> Modified:
> llvm/trunk/include/llvm/ADT/Optional.h
>
> Modified: llvm/trunk/include/llvm/ADT/Optional.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/Optional.h?rev=171413&r1=171412&r2=171413&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/Optional.h (original)
> +++ llvm/trunk/include/llvm/ADT/Optional.h Wed Jan 2 15:19:08 2013
> @@ -28,7 +28,7 @@
> template<typename T>
> class Optional {
> T x;
> - unsigned hasVal : 1;
> + bool hasVal;
> public:
> explicit Optional() : x(), hasVal(false) {}
> Optional(const T &y) : x(y), hasVal(true) {}
Your patch obviously is the better one, which is why I didn't submit my
"unsuitable" one.
Grüße,
Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130104/ebe5682c/attachment-0001.sig>
More information about the llvm-commits
mailing list