- User Since
- May 15 2019, 3:25 PM (80 w, 1 d)
Fri, Nov 20
Thu, Nov 19
Simplify test cases as @MaskRay suggested.
Tue, Nov 17
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.
Mon, Nov 16
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?
Fri, Nov 13
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.
Jun 29 2020
Jun 9 2020
Jun 8 2020
Jun 7 2020
If we have a mechanism bisecting pragmas, this option will not be needed.
Jun 2 2020
@jfb Does the current implementation look good? Thanks.
@efriedma Does the current version of the patch look like it is good to go? Thanks.
May 28 2020
Updates based on comments.
Fix a typo.
Do not create AArch64MCExpr.
Revert to the implementation before the last version.
May 27 2020
May 26 2020
Add the expression operand as AArch64MCExpr.
Fix a typo.
Update based on comments.
May 20 2020
Just want to follow up and see if anything is missing from the latest patch :)