- User Since
- May 15 2019, 3:25 PM (88 w, 3 d)
Thu, Jan 14
The test failure turned out to be a false positive. Relanded the change on https://github.com/llvm/llvm-project/commit/9dfeec853008109b1cbe926c22675c96226040d9
I have mentioned that the GNU objcopy/strip smart_rename still has vulnerability and the owner preservation does not work for non-root (CAP_CHOWN) users.
But I think we should do better: instead of: (1) create a temp file (2) open it by name again (3) check it is a regular file with one hard link & it is writable (S_IWUSR) (4) fstat (5) fchown,
we should just modify the destination in place (for objcopy file and strip file). If the file has been mapped readonly, on many platforms it may be unwritable. We have to accept this.
Wed, Jan 13
Fri, Jan 8
GNU objcopy before 2.36 had the symlink attach vulnerability. The patch would introduce the similar vulnerability to llvm-objcopy.
Wed, Jan 6
Address some comments.
Tue, Jan 5
Dec 20 2020
Dec 17 2020
Update the test case.
Dec 4 2020
Updated a test case.
Update the syntax in the comment.
Address @jrtc27's comment. paseOp function now skips all the spaces between string arguments until it hits any comma or end of statement. For now we will limit the support to .ascii only.
Dec 3 2020
Nov 30 2020
Since GNU assembler changed how it interpreted using space as separators in .asciz and .string in https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=3d955acb36f483c05724181da5ffba46b1303c43, limit this patch to .ascii only as matching the new GAS behavior in the two directives may cause subtle issues when building existing code that assumes the old behavior with LLVM's integrated assembler.
Nov 20 2020
Nov 19 2020
Simplify test cases as @MaskRay suggested.
Nov 17 2020
I think rejecting .asciz "a" "b" may be fine for us since there is no real world usage.
Then I am not sure we need a new method parseManyWithOptionalComma. It can be specialized.
Thanks all for the comments. From what I read so far, we agree thatt there is a real need to implement this feature in LLVM so add more test cases based on the comments.
Nov 16 2020
https://github.com/ClangBuiltLinux/linux/blob/f01c30de86f1047e9bae1b1b1417b0ce8dcd15b1/arch/arm/probes/kprobes/test-core.h#L114-L116 makes it sounds like .ascii and .asciz work differently in this regard?
Nov 13 2020
In which case that's confusing and likely to lead to bugs if people make use of the preprocessor in the hopes of concatenating strings but end up with NUL bytes being inserted contrary to what there would be in C. Can we not just fix the assembly to use commas rather than this weird syntax that seems to be a special case for .asciz? You can't write .word 2 2, only .word 2, 2, so why do string directives really need special treatment?
In which case I assume .asciz "foo" "bar" is equivalent to .asciz "foobar" rather than .asciz "foo\0bar" (same for .string), just like C string concatenation, and hence why both syntaxes exist?
Update commit message and add a test case.
It appears that I misunderstood the existing code. It actually supports multiple string arguments, but only when commas are used between the arguments. GAS supports commas or spaces as the separator (or when both are used in the same call such as .ascii "aa", "bb" "cc ). Will refactor my patch and update the commit message.
Sep 9 2020
Sep 2 2020
Moved the definition of SubsectionNumber to avoid padding bytes. Also updated the comments.
Sep 1 2020
Address @psmith's comments.
Aug 31 2020
Address more comments.
Aug 28 2020
Address @nickdesaulniers' comments.
Aug 27 2020
Combine test cases into one file.
Aug 24 2020
Address some of @MaskRay's comments.
Aug 18 2020
Set the subsection number for fragments in subsecitons.
(1) Fixed a bug while rebasing the patch last time. The failed unit tests dropped to 2 after the fix.
(2) Updated the 2 failed unit tests.
Aug 17 2020
Limit the scope to fragments in the same subsection (fragments not in any subsections are placed in subsection 0). Fragments are inserted into each subsection in order and the difference of their offsets should be fixed once they are inserted. This is experimental as it breaks many tests. Will address the test failure if this idea is proven correct.
Aug 11 2020
Aug 10 2020
Aug 9 2020
Aug 5 2020
@jyknight while working on a different issue, I happened to take a look at the implementation of subsection (MCSection::getSubsectionInsertionPoint) and from what I understood, fragments in subsections were always placed after fragments not in any subsection., and fragments in the same subsection (or not in any subsection) were always inserted in order and separate from fragments in other subsections. Is it safe to assume what between two fragments either not in any subsections or in the same subsection will be remain unchanged once they are inserted to the fragment list of a section? If so, can we resolve differences of two labels in fragments with such restriction?
Aug 3 2020
Jul 31 2020
Renamed a variable name and addressed comments. @craig.topper Please let me know if it looks any better. And thanks for enabling multibyte-NOPs in 64-bit mode.
Jul 23 2020
Jul 15 2020
Jul 7 2020
Fixed an assertion message.
Address some of @reames's concerns, and rename variables to avoid confusion.
Jul 6 2020
Issuing multiple-byte NOP for 32-bit mode broke many tests and probably worth a separate patch itself. So this patch will keep the behavior on 32-bit mode unchange for now. This built and passed all the tests.
Jul 1 2020
Refactor X86AsmBackend::getMaximumNopSize based on @craig.topper's comment.
(1) This is a temporary implementation as it breaks the following tests:
Thanks! Checking if the CPU is 64-bit along FeatureNOPL seemed to work. Will verify more. Also I'd like to point out the difference of 32-bit CPUs.
Jun 30 2020
@craig.topper I tried to add a new Fragment type and use X86AsmBackend::writeNopData to insert NOP instructions. It however changed the outcome of the test cases I used, as X86AsmBackend::writeNopData only generatssingle-byte nop instructions if X86::FeatureNOPL feature bit is not true, while the maxLongNopLength in the current implementation (from X86MCInstLower) allows long nop instructions up to 10 bytes on a 64-bit processor. Which one is preferred? Also, which target triple should I use to test multiple-byte nop instructions? Thanks.