Page MenuHomePhabricator

jcai19 (Jian Cai)
User

Projects

User does not belong to any projects.

User Details

User Since
May 15 2019, 3:25 PM (99 w, 5 d)

Recent Activity

Yesterday

jcai19 committed rGed1734931aa9: Fix up build failures after cfce5b26a888cb979d65252275df1f977dc1e6c8 (authored by jcai19).
Fix up build failures after cfce5b26a888cb979d65252275df1f977dc1e6c8
Mon, Apr 12, 2:09 PM
jcai19 added a comment to D98916: [ARM] support symbolic expression as immediate in memory instructions.

This broke the ppc64le-sanitizer builderr: https://lab.llvm.org/buildbot/#/builders/19/builds/3565. Will fix it.

Mon, Apr 12, 1:49 PM · Restricted Project
jcai19 committed rGcfce5b26a888: [ARM] support symbolic expression as immediate in memory instructions (authored by jcai19).
[ARM] support symbolic expression as immediate in memory instructions
Mon, Apr 12, 12:14 PM
jcai19 closed D98916: [ARM] support symbolic expression as immediate in memory instructions.
Mon, Apr 12, 12:14 PM · Restricted Project

Fri, Apr 9

jcai19 added a comment to D98916: [ARM] support symbolic expression as immediate in memory instructions.

Thanks for the update LGTM. It sounds like you have all the cases covered. Apologies for the noise.

Fri, Apr 9, 11:10 AM · Restricted Project

Thu, Apr 8

jcai19 added inline comments to D98916: [ARM] support symbolic expression as immediate in memory instructions.
Thu, Apr 8, 11:56 AM · Restricted Project
jcai19 updated the diff for D98916: [ARM] support symbolic expression as immediate in memory instructions.

Fix a typo.

Thu, Apr 8, 11:52 AM · Restricted Project
jcai19 added a comment to D98916: [ARM] support symbolic expression as immediate in memory instructions.

I have a one major concern, and a number of minor nits that could easily be addressed prior to commit if necessary.

The major concern is that if the intent is to just permit expressions for Arm state LDR instructions (addition of one fixup) then a lot of code in the AsmParser looks like it is permitting expressions for Thumb and Thumb2 LDR instructions that wouldn't use the one new fixup. Can you double check the predicates to make sure that they apply to the instructions you want to permit expressions on? It is possible I've got something wrong though. If I'm right then where we would have had errors had someone use an expression then we may get an internal error or crash as the backend won't be able to handle the expression. My suggestion would be to take out from ARMAsmParser all the code permitting exceptions except for the ARM instructions that support the fixup. Adding support for expressions to all the ARM and Thumb instructions is possible but that would probably be better done over several patches. If I'm wrong please ignore me and sorry for the noise.

Thu, Apr 8, 11:49 AM · Restricted Project
jcai19 updated the diff for D98916: [ARM] support symbolic expression as immediate in memory instructions.

Address comments.

Thu, Apr 8, 11:48 AM · Restricted Project

Tue, Apr 6

jcai19 added a comment to D98916: [ARM] support symbolic expression as immediate in memory instructions.

I'm ok with this, and thanks for taking the time to work on it. It would be good to collect another LGTM from another reviewer before submitting.

Tue, Apr 6, 12:06 PM · Restricted Project

Mon, Apr 5

jcai19 added inline comments to D98916: [ARM] support symbolic expression as immediate in memory instructions.
Mon, Apr 5, 5:17 PM · Restricted Project
jcai19 updated the diff for D98916: [ARM] support symbolic expression as immediate in memory instructions.

Only accept MCExpr in the affected instructions.

Mon, Apr 5, 5:11 PM · Restricted Project

Thu, Apr 1

jcai19 added a comment to D99556: Add support to -Wa,--version in clang.

Relanded without -fno-initegrated-as case at https://reviews.llvm.org/rG76d9bc72784d88f4dd57b9939e52c73739438af5.

Thu, Apr 1, 1:49 PM · Restricted Project
jcai19 committed rG76d9bc72784d: Reland "Add support to -Wa,--version in clang"" (authored by jcai19).
Reland "Add support to -Wa,--version in clang""
Thu, Apr 1, 1:48 PM

Wed, Mar 31

jcai19 added a comment to D99556: Add support to -Wa,--version in clang.

We can probably remove the -fno-integraetd-as case.

Wed, Mar 31, 10:12 PM · Restricted Project
jcai19 committed rG3cc3c0f8352e: Add support to -Wa,--version in clang (authored by jcai19).
Add support to -Wa,--version in clang
Wed, Mar 31, 4:30 PM
jcai19 closed D99556: Add support to -Wa,--version in clang.
Wed, Mar 31, 4:30 PM · Restricted Project

Tue, Mar 30

jcai19 added inline comments to D99556: Add support to -Wa,--version in clang.
Tue, Mar 30, 6:03 PM · Restricted Project
jcai19 updated the diff for D99556: Add support to -Wa,--version in clang.

Update tests.

Tue, Mar 30, 6:02 PM · Restricted Project
jcai19 updated the diff for D99556: Add support to -Wa,--version in clang.

Update tests to address comments.

Tue, Mar 30, 3:00 PM · Restricted Project

Mon, Mar 29

jcai19 updated subscribers of D99556: Add support to -Wa,--version in clang.
Mon, Mar 29, 7:02 PM · Restricted Project
jcai19 added a reviewer for D99556: Add support to -Wa,--version in clang: nickdesaulniers.
Mon, Mar 29, 6:10 PM · Restricted Project
jcai19 requested review of D99556: Add support to -Wa,--version in clang.
Mon, Mar 29, 6:05 PM · Restricted Project

Thu, Mar 25

jcai19 added inline comments to D98916: [ARM] support symbolic expression as immediate in memory instructions.
Thu, Mar 25, 12:04 PM · Restricted Project

Mon, Mar 22

jcai19 added inline comments to D98916: [ARM] support symbolic expression as immediate in memory instructions.
Mon, Mar 22, 5:52 PM · Restricted Project
jcai19 added inline comments to D98916: [ARM] support symbolic expression as immediate in memory instructions.
Mon, Mar 22, 12:09 PM · Restricted Project
jcai19 updated the diff for D98916: [ARM] support symbolic expression as immediate in memory instructions.

Address comments.

Mon, Mar 22, 12:08 PM · Restricted Project

Fri, Mar 19

jcai19 updated the summary of D98916: [ARM] support symbolic expression as immediate in memory instructions.
Fri, Mar 19, 3:11 PM · Restricted Project
jcai19 added a comment to D98916: [ARM] support symbolic expression as immediate in memory instructions.

The fixup code looks reasonable to me as it is mostly reusing the same logic as the pc-relative fixup. The names are internal to LLVM so there isn't any particular standard. I think this fixup would correspond to the relocation R_ARM_ABS12 https://github.com/ARM-software/abi-aa/blob/master/aaelf32/aaelf32.rst#relocation-codes so it may be worth using fixup_arm_ldst_abs_12 but that is only a suggestion. I wouldn't expect the assembler to use the relocation as the range is so short that any attempt to use it would likely result in an out of range error.

Fri, Mar 19, 3:10 PM · Restricted Project
jcai19 updated the diff for D98916: [ARM] support symbolic expression as immediate in memory instructions.

Address comments so far.

Fri, Mar 19, 3:09 PM · Restricted Project

Thu, Mar 18

jcai19 edited reviewers for D98916: [ARM] support symbolic expression as immediate in memory instructions, added: nickdesaulniers; removed: nickdepinet.
Thu, Mar 18, 8:29 PM · Restricted Project
jcai19 updated subscribers of D98916: [ARM] support symbolic expression as immediate in memory instructions.
Thu, Mar 18, 8:29 PM · Restricted Project
jcai19 added reviewers for D98916: [ARM] support symbolic expression as immediate in memory instructions: peter.smith, psmith, ostannard, DavidSpickett, nickdepinet, MaskRay.
Thu, Mar 18, 8:27 PM · Restricted Project
jcai19 requested review of D98916: [ARM] support symbolic expression as immediate in memory instructions.
Thu, Mar 18, 8:24 PM · Restricted Project

Mar 12 2021

jcai19 added a comment to D98511: [llvm-objcopy][NFC] Move ownership keeping code into restoreStatOnFile().

I'm not sure if this would be a big issue but restoreStatOnFile also runs on the split dwo file, and will keep its ownership with this change. I don't think GNU assembler does that currently. Also you could delete the code added in https://reviews.llvm.org/D98511 for passing user and group ID since they are no longer needed, assuming the new behavior is what we want.

Mar 12 2021, 11:00 AM · Restricted Project

Mar 2 2021

jcai19 added inline comments to D97568: [ARM] support symbolic expressions as branch target in b.w.
Mar 2 2021, 5:57 PM · Restricted Project

Mar 1 2021

jcai19 committed rGc35105055ee4: [ARM] support symbolic expressions as branch target in b.w (authored by jcai19).
[ARM] support symbolic expressions as branch target in b.w
Mar 1 2021, 5:42 PM
jcai19 closed D97568: [ARM] support symbolic expressions as branch target in b.w.
Mar 1 2021, 5:42 PM · Restricted Project
jcai19 updated the diff for D97568: [ARM] support symbolic expressions as branch target in b.w.

Add period after a comment.

Mar 1 2021, 3:06 PM · Restricted Project
jcai19 updated the diff for D97568: [ARM] support symbolic expressions as branch target in b.w.

Remove unncessary CHECK-NOT.

Mar 1 2021, 2:46 PM · Restricted Project
jcai19 abandoned D97501: [ARM] support symbolic expressions as branch target.
Mar 1 2021, 2:07 PM · Restricted Project
jcai19 added inline comments to D97501: [ARM] support symbolic expressions as branch target.
Mar 1 2021, 2:07 PM · Restricted Project
jcai19 updated the diff for D97501: [ARM] support symbolic expressions as branch target.

Update test cases.

Mar 1 2021, 2:01 PM · Restricted Project

Feb 26 2021

jcai19 updated the diff for D97568: [ARM] support symbolic expressions as branch target in b.w.

Skip range check of MCBinaryExpr in b.w instead of any non-constant expressions.

Feb 26 2021, 4:58 PM · Restricted Project
jcai19 added inline comments to D97501: [ARM] support symbolic expressions as branch target.
Feb 26 2021, 4:41 PM · Restricted Project
jcai19 updated the diff for D97501: [ARM] support symbolic expressions as branch target.

Skip range check of MCBinaryExpr instead of any non-constant expressions.

Feb 26 2021, 4:40 PM · Restricted Project
jcai19 added inline comments to D97501: [ARM] support symbolic expressions as branch target.
Feb 26 2021, 11:33 AM · Restricted Project
jcai19 added reviewers for D97568: [ARM] support symbolic expressions as branch target in b.w: DavidSpickett, psmith, ostannard, eli.friedman, nickdesaulniers.
Feb 26 2021, 11:33 AM · Restricted Project
jcai19 added a comment to D97568: [ARM] support symbolic expressions as branch target in b.w.

This is created based on the discussion at https://reviews.llvm.org/D97501#inline-914358.

Feb 26 2021, 11:26 AM · Restricted Project
jcai19 requested review of D97568: [ARM] support symbolic expressions as branch target in b.w.
Feb 26 2021, 11:25 AM · Restricted Project

Feb 25 2021

jcai19 added inline comments to D97501: [ARM] support symbolic expressions as branch target.
Feb 25 2021, 11:38 PM · Restricted Project
jcai19 updated the diff for D97501: [ARM] support symbolic expressions as branch target.

Remove encodings from the tests.

Feb 25 2021, 6:17 PM · Restricted Project
jcai19 updated the diff for D97501: [ARM] support symbolic expressions as branch target.

Update the test cases.

Feb 25 2021, 4:43 PM · Restricted Project
jcai19 added reviewers for D97501: [ARM] support symbolic expressions as branch target: DavidSpickett, psmith, ostannard, eli.friedman, nickdesaulniers.
Feb 25 2021, 2:43 PM · Restricted Project
jcai19 requested review of D97501: [ARM] support symbolic expressions as branch target.
Feb 25 2021, 2:32 PM · Restricted Project

Feb 17 2021

jcai19 closed D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

Landed at https://reviews.llvm.org/rGc2a84771bb63947695ea50b89160c02b36fb634d.

Feb 17 2021, 12:32 PM · Restricted Project

Feb 12 2021

jcai19 committed rGc2a84771bb63: [llvm-objcopy] preserve file ownership when overwritten by root (authored by jcai19).
[llvm-objcopy] preserve file ownership when overwritten by root
Feb 12 2021, 6:02 PM

Feb 11 2021

jcai19 updated the summary of D93881: [llvm-objcopy] preserve file ownership when overwritten by root.
Feb 11 2021, 6:51 PM · Restricted Project
jcai19 updated the diff for D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

Update a comment.

Feb 11 2021, 5:59 PM · Restricted Project
jcai19 added a comment to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

If you use arc diff 'HEAD^', you need --verbatim to amend the description.

Gotcha. Thanks. I've updated the message.

Feb 11 2021, 5:57 PM · Restricted Project
jcai19 retitled D93881: [llvm-objcopy] preserve file ownership when overwritten by root from [llvm-objcopy] preserve file ownership when overwritten to [llvm-objcopy] preserve file ownership when overwritten by root.
Feb 11 2021, 5:55 PM · Restricted Project
jcai19 added a comment to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

Overall, LGTM. I am fine with fchown, but the description needs a lot of changes to proceed.

Feb 11 2021, 5:17 PM · Restricted Project

Feb 9 2021

jcai19 added a comment to D96308: [llvm-objcopy] Avoid rename if input filename = output filename.

Thanks for this patch. You mentioned the following issue in one of your comments on https://reviews.llvm.org/D93881. IIUC, this patch seems to not have taken care of this issue.

Feb 9 2021, 2:01 PM · Restricted Project

Feb 5 2021

jcai19 added inline comments to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.
Feb 5 2021, 12:55 PM · Restricted Project
jcai19 updated the diff for D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

Reuse the result of stat instead of calling it on the output file again.

Feb 5 2021, 12:55 PM · Restricted Project

Feb 4 2021

jcai19 added inline comments to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.
Feb 4 2021, 8:47 PM · Restricted Project
jcai19 updated the diff for D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

Check the user of the temporary file instead of the output file to decide the user that invokes the tool.

Feb 4 2021, 8:40 PM · Restricted Project
jcai19 added inline comments to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.
Feb 4 2021, 7:24 PM · Restricted Project
jcai19 updated the diff for D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

Limit the patch to root user.

Feb 4 2021, 7:19 PM · Restricted Project
jcai19 added inline comments to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.
Feb 4 2021, 6:48 PM · Restricted Project
jcai19 added inline comments to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.
Feb 4 2021, 6:43 PM · Restricted Project

Feb 3 2021

jcai19 added inline comments to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.
Feb 3 2021, 7:46 PM · Restricted Project
jcai19 updated the diff for D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

Add trailing comma to comments.

Feb 3 2021, 7:40 PM · Restricted Project

Feb 2 2021

jcai19 updated the diff for D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

Address @manojgupta's comments.

Feb 2 2021, 10:32 PM · Restricted Project
jcai19 updated the diff for D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

Update the ownership of the temporary file, instead of the overwritten file.

Feb 2 2021, 8:28 PM · Restricted Project

Jan 14 2021

jcai19 closed D91460: [AsmParser] make .ascii support spaces as separators.

The test failure turned out to be a false positive. Relanded the change on https://github.com/llvm/llvm-project/commit/9dfeec853008109b1cbe926c22675c96226040d9

Jan 14 2021, 5:55 PM · Restricted Project
jcai19 committed rG9dfeec853008: Reland "[AsmParser] make .ascii support spaces as separators" (authored by jcai19).
Reland "[AsmParser] make .ascii support spaces as separators"
Jan 14 2021, 5:53 PM
jcai19 added a comment to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

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.

Jan 14 2021, 5:27 PM · Restricted Project
jcai19 added a comment to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

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.

Jan 14 2021, 3:15 PM · Restricted Project

Jan 13 2021

jcai19 added a comment to D91460: [AsmParser] make .ascii support spaces as separators.

Which GDB tests? How are they broken? I'm extremely surprised by the given this patch implements what binutils has done for a very long time (only .asciz/.string have changed behaviour in binutils, and those are not modified by this patch here so should not have regressions).

Jan 13 2021, 3:04 PM · Restricted Project
jcai19 added a reverting change for rGe0963ae274be: [AsmParser] make .ascii support spaces as separators: rG82c4153e66fa: Revert "[AsmParser] make .ascii support spaces as separators".
Jan 13 2021, 2:39 PM
jcai19 committed rG82c4153e66fa: Revert "[AsmParser] make .ascii support spaces as separators" (authored by jcai19).
Revert "[AsmParser] make .ascii support spaces as separators"
Jan 13 2021, 2:39 PM
jcai19 added a reverting change for D91460: [AsmParser] make .ascii support spaces as separators: rG82c4153e66fa: Revert "[AsmParser] make .ascii support spaces as separators".
Jan 13 2021, 2:38 PM · Restricted Project

Jan 8 2021

jcai19 added a comment to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

From a user standpoint, I agree that "objcopy foo.o" should not be changing the ownership of a file, regardless of the implementation (which, in the case of llvm-objcopy, is getting it "wrong" compared to GNU objcopy, because we use temporary file and rename it at the end, without fixing ownership). Sounds like there's agreement there.

Jan 8 2021, 11:56 AM · Restricted Project
jcai19 added a comment to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

GNU objcopy before 2.36 had the symlink attach vulnerability. The patch would introduce the similar vulnerability to llvm-objcopy.

Jan 8 2021, 11:43 AM · Restricted Project

Jan 6 2021

jcai19 added inline comments to D93881: [llvm-objcopy] preserve file ownership when overwritten by root.
Jan 6 2021, 2:49 PM · Restricted Project
jcai19 updated the diff for D93881: [llvm-objcopy] preserve file ownership when overwritten by root.

Address some comments.

Jan 6 2021, 2:48 PM · Restricted Project

Jan 5 2021

jcai19 published D93881: [llvm-objcopy] preserve file ownership when overwritten by root for review.
Jan 5 2021, 9:27 PM · Restricted Project

Dec 20 2020

jcai19 committed rGe0963ae274be: [AsmParser] make .ascii support spaces as separators (authored by jcai19).
[AsmParser] make .ascii support spaces as separators
Dec 20 2020, 10:45 PM
jcai19 closed D91460: [AsmParser] make .ascii support spaces as separators.
Dec 20 2020, 10:45 PM · Restricted Project

Dec 17 2020

jcai19 added a comment to D91460: [AsmParser] make .ascii support spaces as separators.

Looks good to me, let's see what the others think.

Dec 17 2020, 2:59 PM · Restricted Project
jcai19 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 17 2020, 2:41 PM · Restricted Project
jcai19 updated the diff for D91460: [AsmParser] make .ascii support spaces as separators.

Update the test case.

Dec 17 2020, 2:11 PM · Restricted Project
jcai19 added a comment to D91460: [AsmParser] make .ascii support spaces as separators.

Just want to follow up and see if anyone might have any thoughts on the current implementation of this patch. Otherwise, is it good to submit the patch? @jrtc27 @rnk @nickdesaulniers @MaskRay

Dec 17 2020, 1:37 PM · Restricted Project

Dec 4 2020

jcai19 updated the diff for D91460: [AsmParser] make .ascii support spaces as separators.

Updated a test case.

Dec 4 2020, 3:54 PM · Restricted Project
jcai19 updated the diff for D91460: [AsmParser] make .ascii support spaces as separators.

Update the syntax in the comment.

Dec 4 2020, 3:19 PM · Restricted Project
jcai19 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 4 2020, 3:18 PM · Restricted Project
jcai19 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 4 2020, 3:03 PM · Restricted Project
jcai19 updated the diff for D91460: [AsmParser] make .ascii support spaces as separators.

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 4 2020, 2:23 PM · Restricted Project

Dec 3 2020

jcai19 added inline comments to D91460: [AsmParser] make .ascii support spaces as separators.
Dec 3 2020, 5:32 PM · Restricted Project