This revision was automatically updated to reflect the committed changes.
Closed by commit rC339668: [gnu-objc] Make selector order deterministic.
(authored by theraven, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D50559?vs=160312=160545#toc
Repository:
rC Clang
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.
Thanks. I appreciate the fact that you spelled it all out in the test, too.
LGTM.
Comment at: lib/CodeGen/CGObjCGNU.cpp:3547
+
theraven updated this revision to Diff 160312.
theraven added a comment.
- Add a test case.
Repository:
rC Clang
https://reviews.llvm.org/D50559
Files:
lib/CodeGen/CGObjCGNU.cpp
test/CodeGenObjC/gnu-deterministic-selectors.m
Index: test/CodeGenObjC/gnu-deterministic-selectors.m
bmwiedemann added a comment.
I got it compiled with std::sort on top of 6.0.1 and found that it indeed
removes the indeterminism: When I compiled libobjc2-1.8.1/arc.m 25 times, I got
the same md5sum every time - and the same result with and without ASLR.
Repository:
rC Clang
theraven added inline comments.
Comment at: lib/CodeGen/CGObjCGNU.cpp:3547
+ allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
bmwiedemann wrote:
> compilation failed here:
>
rjmccall added a comment.
You can't test that there's no non-determinism, but you can certainly test that
we emit selectors in sorted order as opposed to the order in which they're
used. I imagine a function with a bunch of `@selector` expressions should be
good enough for that.
Repository:
mgrang added a comment.
In https://reviews.llvm.org/D50559#1195787, @bmwiedemann wrote:
> got a build failure with this patch added onto 6.0.1
You may have to add:
#include "llvm/ADT/STLExtras.h"
Repository:
rC Clang
https://reviews.llvm.org/D50559
bmwiedemann added a comment.
got a build failure with this patch added onto 6.0.1
Comment at: lib/CodeGen/CGObjCGNU.cpp:3547
+ allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
compilation failed here:
mgrang added a comment.
Does the error show up if you build llvm with -DLLVM_REVERSE_ITERATION:ON?
LLVM_REVERSE_ITERATION:BOOL
If enabled, all supported unordered llvm containers would be iterated in
reverse order. This is useful for uncovering non-determinism caused by
iteration of
theraven created this revision.
theraven added a reviewer: rjmccall.
Herald added subscribers: cfe-commits, mgrang.
This probably fixes PR35277, though there may be other sources of
nondeterminism (this was the only case of iterating over a DenseMap).
It's difficult to provide a test case for
10 matches
Mail list logo
