[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-05-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 199926. jcai19 added a comment. Use non-const& type for readbility. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61967/new/ https://reviews.llvm.org/D61967 Files:

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-05-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 2 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:27 +void CloexecPipeCheck::check(const MatchFinder::MatchResult ) { + const std::string = + (Twine("pipe2(") +

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 created this revision. Herald added subscribers: cfe-commits, xazax.hun, mgorny, srhines. Herald added a project: clang. pipe2() is better to set O_CLOEXEC flag to avoid file descriptor leakage. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D62049 Files:

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-05-15 Thread Jian Cai via Phabricator via cfe-commits
jcai19 created this revision. jcai19 added reviewers: alexfh, chh. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. pipe() is better to be replaced by pipe2() with O_CLOEXEC flag to avoid file descriptor leakage. Repository: rG LLVM Github Monorepo

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-17 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 200066. jcai19 added a comment. Update based on comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62049/new/ https://reviews.llvm.org/D62049 Files:

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-05-17 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 200068. jcai19 marked an inline comment as done. jcai19 added a comment. Update based on comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61967/new/ https://reviews.llvm.org/D61967 Files:

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-06-02 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 6 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:31 + Result, + "prefer pipe2() to pipe() because pipe2() allows O_CLOEXEC", + ReplacementText); gribozavr

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-06-02 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 202625. jcai19 marked an inline comment as done. jcai19 added a comment. Remove CHECK-MESSAGES-NOT and update description based on comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61967/new/

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-06-04 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 202984. jcai19 added a comment. Update test function names. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61967/new/ https://reviews.llvm.org/D61967 Files:

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-06-04 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:20 + pipe2(pipefd, O_NONBLOCK); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'pipe2' should use O_CLOEXEC where possible

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-06-04 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:17 + +void warning1() { + int pipefd[2]; gribozavr wrote: > `warning1`, `warning2` is better than `a` and `b`, but we

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-06-04 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 203050. jcai19 added a comment. Update test function names. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62049/new/ https://reviews.llvm.org/D62049 Files:

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-06-03 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 202830. jcai19 added a comment. Update description based on comments and remove CHECK-MESSAGES-NOT Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62049/new/ https://reviews.llvm.org/D62049 Files:

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-06-03 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 4 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:20 + pipe2(pipefd, O_NONBLOCK); + // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 'pipe2' should use O_CLOEXEC where possible

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-06-03 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 202832. jcai19 added a comment. Fix a typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62049/new/ https://reviews.llvm.org/D62049 Files: clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-06-03 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 202835. jcai19 added a comment. Update the test function names. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61967/new/ https://reviews.llvm.org/D61967 Files:

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-06-03 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added a comment. Thanks for all the comments. george.burgess.iv will submit this change for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61967/new/ https://reviews.llvm.org/D61967

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-29 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 202072. jcai19 marked an inline comment as done. jcai19 added a comment. Remove redundant CHECK-MESSAGES-NOT checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62049/new/ https://reviews.llvm.org/D62049

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-29 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 2 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:52 + +void e() { + int pipefd[2]; srhines wrote: > jcai19 wrote: > > srhines wrote: > > > I'm not all that familiar

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-29 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 202030. jcai19 added a comment. Add CHECK-MESSAGES-NOT checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62049/new/ https://reviews.llvm.org/D62049 Files:

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-05-29 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 202052. jcai19 added a comment. Add CHECK-MESSAGES-NOT checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61967/new/ https://reviews.llvm.org/D61967 Files:

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-29 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. In D62049#1522203 , @srhines wrote: > Everything looks great. Thanks for adding these improvements. While it's > probably safe to commit this, perhaps you should give it 24 hours in case > some of the other clang-tidy folks have

[PATCH] D63623: [clang-tidy] Add a check on expected return values of posix functions (except posix_openpt) in Android module.

2019-06-24 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 206273. jcai19 added a comment. Fix typos and formatting issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/ https://reviews.llvm.org/D63623 Files:

[PATCH] D63623: [clang-tidy] Add a check on expected return values of posix functions (except posix_openpt) in Android module.

2019-06-24 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 10 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/clang-tidy/android/PosixReturnCheck.cpp:23 + binaryOperator( + hasOperatorName("<"), + hasLHS(callExpr(callee(functionDecl(matchesName("^::posix_"),

[PATCH] D63623: [clang-tidy] Add a check on expected return values of posix functions (except posix_openpt) in Android module.

2019-06-24 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. In D63623#1556161 , @lebedev.ri wrote: > In D63623#1552716 , @jcai19 wrote: > > > In D63623#1552679 , @lebedev.ri > > wrote: > > > > > Why is this

[PATCH] D63623: [clang-tidy] Add a check on expected return values of posix functions (except posix_openpt) in Android module.

2019-06-20 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. In D63623#1552679 , @lebedev.ri wrote: > Why is this in android module? > Is that android-specific behavior, or posix? I implemented it for Andriod as requested, but it would be completely fine to move it if it is better to

[PATCH] D63623: Add a check on expected return values of posix functions (except posix_openpt) in Android module.

2019-06-20 Thread Jian Cai via Phabricator via cfe-commits
jcai19 created this revision. Herald added subscribers: cfe-commits, mgorny, srhines. Herald added a project: clang. Checks if any calls to posix functions (except posix_openpt) expect negative return values. These functions return either 0 on success or an errno on failure, which is positive

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-05-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 199870. jcai19 marked 4 inline comments as done. jcai19 added a comment. Fix format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61967/new/ https://reviews.llvm.org/D61967 Files:

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-05-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 4 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/clang-tidy/android/CloexecPipeCheck.cpp:27 +void CloexecPipeCheck::check(const MatchFinder::MatchResult ) { + const std::string = + (Twine("pipe2(") +

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-05-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 2 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe.cpp:9 + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: prefer pipe2() to pipe() because pipe2() allows O_CLOEXEC [android-cloexec-pipe] + //

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-21 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 2 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe2.rst:19 + + pipe2(pipefd, O_CLOEXEC); srhines wrote: > Shouldn't this be "O_NONBLOCK | O_CLOEXEC" instead? Why drop

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-21 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 200547. jcai19 added a comment. Fix a typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62049/new/ https://reviews.llvm.org/D62049 Files: clang-tools-extra/clang-tidy/android/AndroidTidyModule.cpp

[PATCH] D62049: [clang-tidy] Add a close-on-exec check on pipe2() in Android module.

2019-05-21 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 2 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/android-cloexec-pipe2.cpp:52 + +void e() { + int pipefd[2]; srhines wrote: > I'm not all that familiar with writing clang-tidy-specific tests,

[PATCH] D61967: [clang-tidy] Add a close-on-exec check on pipe() in Android module.

2019-05-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 199861. jcai19 added a comment. Add fixes based on comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61967/new/ https://reviews.llvm.org/D61967 Files:

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

2019-06-27 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 2 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/test/clang-tidy/bugprone-posix-return.cpp:96 + if (posix_fadvise(0, 0, 0, 0) < 0) {} + if (posix_fadvise(0, 0, 0, 0) >= 0) {} else {} + if (posix_fadvise(0, 0, 0, 0) == -1) {}

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

2019-06-27 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 206947. jcai19 added a comment. Update test names. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/ https://reviews.llvm.org/D63623 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

2019-06-27 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 206946. jcai19 added a comment. Update the check to catch redundant code and update the warning message to be more speicific. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

2019-06-26 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 2 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/PosixReturnCheck.cpp:49 +diag(OperatorLoc, "POSIX functions (except posix_openpt) never return negative values") +<<

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

2019-07-02 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. In D63623#1565377 , @gribozavr wrote: > Thanks! Do you have commit access? Do you want me to commit this patch for > you? Totally missed your message yesterday. Sorry about that. I do not have the access so it will be great if

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

2019-07-01 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 207401. jcai19 added a comment. Update CHECK-MESSAGES messages accordingly. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/ https://reviews.llvm.org/D63623 Files:

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

2019-07-01 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 207398. jcai19 marked 2 inline comments as done. jcai19 added a comment. Update warning messages. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/ https://reviews.llvm.org/D63623 Files:

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

2019-07-01 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 207397. jcai19 added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/ https://reviews.llvm.org/D63623 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D63623: [clang-tidy] Add a check on expected return values of posix functions (except posix_openpt) in Android module.

2019-06-26 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added inline comments. Comment at: clang-tools-extra/clang-tidy/android/PosixReturnCheck.cpp:39 + const auto = *Result.Nodes.getNodeAs("binop"); + diag(BinOp.getOperatorLoc(), "posix functions (except posix_openpt) never return

[PATCH] D63623: [clang-tidy] Add a check on expected return values of posix functions (except posix_openpt) in Android module.

2019-06-26 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 206588. jcai19 marked 4 inline comments as done. jcai19 added a comment. Update a test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/ https://reviews.llvm.org/D63623 Files:

[PATCH] D63623: [clang-tidy] Add a check on expected return values of posix functions (except posix_openpt) in Android module.

2019-06-26 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 206586. jcai19 marked an inline comment as done. jcai19 added a comment. Move the check from android to bugprone. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/ https://reviews.llvm.org/D63623 Files:

[PATCH] D63623: [clang-tidy] new check: bugprone-posix-return

2019-06-28 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 207102. jcai19 added a comment. Update warning messages. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63623/new/ https://reviews.llvm.org/D63623 Files:

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-13 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 214884. jcai19 added a comment. Mark LR as live-in at (t)BL_PUSHLR instruction. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files: clang/lib/Basic/Targets/ARM.cpp

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-13 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 4 inline comments as done. jcai19 added inline comments. Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1927 +.add(predOps(ARMCC::AL)) +.addReg(ARM::LR); + efriedma wrote: > I think you need to ensure that lr

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-13 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 214889. jcai19 added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files: clang/lib/Basic/Targets/ARM.cpp

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-12 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added inline comments. Comment at: llvm/test/CodeGen/ARM/gnu_mcount_nc.ll:1-6 +; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - | FileCheck %s --check-prefix=CHECK-ARM +; RUN: llc -mtriple=armv7a-linux-gnueabihf %s -o - |

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-13 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added inline comments. Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1922 + const bool Thumb = Opcode == ARM::tBL_PUSHLR; + MachineOperand ReturnAddr = MI.getOperand(0); + assert(ReturnAddr.getReg() ==

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-13 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 214908. jcai19 added a comment. Fix a typo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files: clang/lib/Basic/Targets/ARM.cpp

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-13 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 214945. jcai19 marked an inline comment as done. jcai19 added a comment. Updates based on comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files:

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-13 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 2 inline comments as done. jcai19 added inline comments. Comment at: llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp:1931 + +// bl__gnu_mcount_nc +MIB = BuildMI(MBB, MBBI, MI.getDebugLoc(), TII->get(ARM::tBL)); nickdesaulniers wrote: >

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-14 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. In D65019#1630354 , @nickdesaulniers wrote: > Great! LGTM and thank you for this patch. Please give 24hrs for > @eli.friedman or @kristof.beyls to leave comments before merging. Sounds good! Thanks for all the comments.

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 closed this revision. jcai19 added a comment. Upsteamed to r369173. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 ___ cfe-commits mailing list

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-12 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. In D65019#1625780 , @efriedma wrote: > > It seems global-isel does not fall back to DAGISel? > > It does, for targets where it's enabled by default, or if you use the right > flags. I think you want `-global-isel

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-12 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 214696. jcai19 added a comment. Add proper instruction selection options to unit test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files:

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-12 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 214698. jcai19 added a comment. Added back an accidently-deleted blank line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files: clang/lib/Basic/Targets/ARM.cpp

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. The changes have been upstreamed to r369147. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D65037: push LR before mcount on ARM

2019-08-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 215703. jcai19 added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fix frontend unit tests for __gnu_mcount_nc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65037/new/

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-16 Thread Jian Cai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL369147: [ARM] push LR before __gnu_mcount_nc (authored by jcai19, committed by ). Changed prior to commit: https://reviews.llvm.org/D65019?vs=214945=215664#toc Repository: rL LLVM CHANGES SINCE

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 215710. jcai19 added a comment. Fix frontend mcount unit tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files: clang/lib/Basic/Targets/ARM.cpp

[PATCH] D66627: [clang-tidy] add checks to bugprone-posix-return

2019-08-26 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 217236. jcai19 added a comment. Update Release Notes and check documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66627/new/ https://reviews.llvm.org/D66627 Files:

[PATCH] D66627: [clang-tidy] new check: bugprone-pthread-return

2019-08-26 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added a comment. In D66627#1644775 , @gribozavr wrote: > Thanks for the contribution! In abstract, I think it is a good checker, > however, the implementation largely duplicates >

[PATCH] D66627: [clang-tidy] new check: bugprone-pthread-return

2019-08-26 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 217231. jcai19 added a comment. Add pthread function calls return values check to bugprone-posix-return. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66627/new/ https://reviews.llvm.org/D66627 Files:

[PATCH] D66627: [clang-tidy] add checks to bugprone-posix-return

2019-08-27 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 217532. jcai19 added a comment. Fix format and test cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66627/new/ https://reviews.llvm.org/D66627 Files:

[PATCH] D66627: [clang-tidy] add checks to bugprone-posix-return

2019-08-27 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 3 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:70 +- New :doc:`bugprone-posix-return + ` check. Eugene.Zelenko wrote: > Check is not new, just modified. Such check should be after new

[PATCH] D66627: [clang-tidy] new check: bugprone-pthread-return

2019-08-22 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 216765. jcai19 added a comment. Fix format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66627/new/ https://reviews.llvm.org/D66627 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D66627: [clang-tidy] new check: bugprone-pthread-return

2019-08-22 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 216768. jcai19 added a comment. Fix typos. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66627/new/ https://reviews.llvm.org/D66627 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D66627: [clang-tidy] new check: bugprone-pthread-return

2019-08-22 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 7 inline comments as done. jcai19 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/PthreadReturnCheck.cpp:2 +//===--- PthreadReturnCheck.cpp - clang-tidy---===//

[PATCH] D66627: [clang-tidy] new check: bugprone-pthread-return

2019-08-22 Thread Jian Cai via Phabricator via cfe-commits
jcai19 created this revision. Herald added subscribers: cfe-commits, jfb, xazax.hun, mgorny. Herald added a project: clang. Checks if any calls to POSIX pthread functions expect negative return values. These functions return either 0 on success or an errno on failure, which is positive only.

[PATCH] D66627: [clang-tidy] add checks to bugprone-posix-return

2019-09-16 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 220393. jcai19 added a comment. Thanks for the confirmation! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66627/new/ https://reviews.llvm.org/D66627 Files:

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-07 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added a comment. In D65019#1619511 , @efriedma wrote: > This seems better. > > I'm not sure I follow why this needs special handling in > SelectionDAGBuilder::visitIntrinsicCall, as opposed to just using >

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

2019-08-06 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 213728. jcai19 added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Introduce a new ARM intrinsic for __gnu_mcount_nc. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

2019-08-06 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. In D65019#1615898 , @efriedma wrote: > > introduce quite some code to the target-independent part of SelectionDAG, > > specifically SelectionDAGBuilder and SelectionDAG classes > > Not sure why you think this would be necessary.

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

2019-08-06 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added inline comments. Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:211 unsigned ARMMoveToIntReg(MVT VT, unsigned SrcReg); -unsigned ARMSelectCallOp(bool UseReg); +unsigned ARMSelectCallOp(bool UseReg, bool PushLR =

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

2019-08-06 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 213762. jcai19 added a comment. Update based on comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files: clang/lib/Basic/Targets/ARM.cpp

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

2019-08-06 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 4 inline comments as done. jcai19 added inline comments. Comment at: llvm/lib/Target/ARM/ARMFastISel.cpp:2418 + bool PushLR = IntrinsicName && + !memcmp(IntrinsicName, "\01__gnu_mcount_nc", sizeof("\01__gnu_mcount_nc")); + unsigned CallOpc =

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc on ARM

2019-08-06 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 213748. jcai19 marked 6 inline comments as done. jcai19 added a comment. Update based on comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files:

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-08 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. In D65019#1621670 , @efriedma wrote: > Yes, it's technically a "call", but you don't need the call lowering code. > That's dedicated to stuff like passing arguments, returning values, checking > whether the function can be

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-08 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added inline comments. Comment at: clang/lib/Basic/Targets/ARM.cpp:325 + ? "llvm.arm.gnu.eabi.mcount" : "\01mcount"; nickdesaulniers wrote: > Doesn't require

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-08 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 214263. jcai19 added a comment. Lower the new intrinsic when legalizing DAGs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files: clang/lib/Basic/Targets/ARM.cpp

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-09 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 214496. jcai19 marked 2 inline comments as done. jcai19 added a comment. clang-format the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019 Files:

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-09 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 4 inline comments as done. jcai19 added a comment. @efriedma I have changed my implementation to lower llvm.gnu.eabi.mcount intrinsic into pseudo instructions directly, instead of first lowering them into SelectionDAG call nodes. Thanks. Comment at:

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

2019-08-09 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 214491. jcai19 added a comment. Lower the intrinsic to pseudo instructions directly, instead of SelectDAG nodes first. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65019/new/ https://reviews.llvm.org/D65019

[PATCH] D68884: Add support to -Wa,-W in clang

2019-10-14 Thread Jian Cai via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4ec5205da701: Add support to -Wa,-W in clang (authored by jcai19). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68884/new/ https://reviews.llvm.org/D68884

[PATCH] D68884: Add support to -Wa,-W in clang

2019-10-14 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 224908. jcai19 added a comment. For autogenerated comment log. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68884/new/ https://reviews.llvm.org/D68884 Files: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D68884: Add support to -Wa,-W in clang

2019-10-11 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 224664. jcai19 added a comment. Update test cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68884/new/ https://reviews.llvm.org/D68884 Files: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D68884: Add support to -Wa,-W in clang

2019-10-11 Thread Jian Cai via Phabricator via cfe-commits
jcai19 created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. jcai19 added a reviewer: bcain. jcai19 added subscribers: nickdesaulniers, manojgupta, llozano. jcai19 updated this revision to Diff 224663. jcai19 added a comment. Fix typos. Currently clang

[PATCH] D68884: Add support to -Wa,-W in clang

2019-10-11 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 224663. jcai19 added a comment. Fix typos. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68884/new/ https://reviews.llvm.org/D68884 Files: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-03-31 Thread Jian Cai via Phabricator via cfe-commits
jcai19 created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Add -ftrivial-auto-var-init-stop-after= to limit the number of times stack variables are initialized when -ftrivial-auto-var-init= is used to initialize stack variables to zero or a pattern. This

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-03-31 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 254019. jcai19 added a comment. Add test cases. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77168/new/ https://reviews.llvm.org/D77168 Files: clang/include/clang/Basic/LangOptions.def

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-03-31 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. Agreed, but having this option may save quite some time when bisecting files with many functions and stack variables, like this one . It will be much easier to write

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-04-01 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 254242. jcai19 added a comment. Remove an unnecessary line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77168/new/ https://reviews.llvm.org/D77168 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-04-01 Thread Jian Cai via Phabricator via cfe-commits
jcai19 updated this revision to Diff 254240. jcai19 added a comment. Address some of the concerns. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77168/new/ https://reviews.llvm.org/D77168 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-04-01 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. > For example, won't you need some way of figuring out what the 1,134th > variable in a file was after you've successfully bisected? Once we know the Xth variable is causing issues, we can compare the IR or assembly of -fvar-auto-init-stop-after=X-1 and

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-04-01 Thread Jian Cai via Phabricator via cfe-commits
jcai19 added a comment. In D77168#1955312 , @jfb wrote: > Do you not think `pragma` is a more general approach? That's what's used in a > bunch of other cases, and I'd like to see it attempted here. Yes I absolutely agree pragma is a great way to

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-04-01 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked 4 inline comments as done. jcai19 added inline comments. Comment at: clang/lib/CodeGen/CGDecl.cpp:1814 +if (StopAfter) { + static unsigned Counter = 0; + if (Counter >= StopAfter) rjmccall wrote: > srhines wrote: > > MaskRay wrote: >

[PATCH] D77168: Add a flag to debug automatic variable initialization

2020-04-28 Thread Jian Cai via Phabricator via cfe-commits
jcai19 marked an inline comment as done. jcai19 added a comment. > - Allows bracketing as John suggested (lower / upper bounds where to stop / > start). I have made some updates to this patch based on the comments here. One question I have though is whether we should have another option for

  1   2   >