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

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 w

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

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/ https://review

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

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: clang-tools-e

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

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 [android-cl

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

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 clang-t

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

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: clang-tools-extra/clang-tidy/android/AndroidTidy

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

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.

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 [android-cl

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

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: clang-tools-extra/clang-tidy/android/AndroidTidyModu

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

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 c

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

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: clang-tools-extra/clang-tidy/android/AndroidTidyModu

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

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

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. Ev

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

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: clang/lib/Bas

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

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

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 llvm/include/llvm/IR

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

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 = ARMSelectCallOp(

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

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

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 tail-c

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

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

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 c

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

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 F

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

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: llvm/lib/Tar

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

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: clang/lib/Basic

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

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 - | FileChe

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

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: clang/lib/Basic/Targets/A

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

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 -global-isel-abor

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

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 l

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

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

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 actual

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

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 llvm/include/llvm/IR/IntrinsicsAR

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

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 llvm/include/llvm/IR/IntrinsicsARM

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

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

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: clang/lib/Ba

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

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() == ARM:

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

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. Repos

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

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&id=215664#toc Repository: rL LLVM CHANGES SINCE L

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

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 htt

[PATCH] D65037: push LR before mcount on ARM

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/ https:

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

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 clang/test/Fro

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

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 cfe-comm

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

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 on

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

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 plac

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

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: clang-tools-extra/clang-tidy/android/AndroidTid

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

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_"), unles

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

jcai19 added a comment. In D63623#1556161 , @lebedev.ri wrote: > In D63623#1552716 , @jcai19 wrote: > > > In D63623#1552679 , @lebedev.ri > > wrote: > > > > > Why is this in

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

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] Add a check on expected return values of posix functions (except posix_openpt) in Android module.

jcai19 marked an inline comment as done. jcai19 added inline comments. Comment at: clang-tools-extra/clang-tidy/android/PosixReturnCheck.cpp:39 + const auto &BinOp = *Result.Nodes.getNodeAs("binop"); + diag(BinOp.getOperatorLoc(), "posix functions (except posix_openpt) never r

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

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: clang-tools-extra/c

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

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") +<< FixItHint::CreateReplacement(

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

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/ https://reviews.llvm.org/D6

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

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

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

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: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModul

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

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 clan

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

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: clang-tools-ex

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

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: clang-tools-extra/clang-tidy/bugpron

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

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 y

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

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 htt

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

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: clang-tools-extra/clang-tidy/android/AndroidTidyModu

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

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: clang-tools-extra/clang-tidy

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

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 &Result) { + const std::string &ReplacementText = + (Twine("pipe2(")

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

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] + // C

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

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: clang-tools-extra/clang-tidy/android/Android

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

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 &Result) { + const std::string &ReplacementText = + (Twine("pipe2(")

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

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: clang-tools-e

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

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: clang-tools-extra/clang-tidy/android/AndroidTidyModule

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

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: clang-tools-e

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

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.

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 clang-t

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

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, bu

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

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 wit

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

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: clang-tools-extra/clang-tidy/android/AndroidTidyM

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

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: clang-tools-extra/clang-tidy/android/AndroidTidyM

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

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 File

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

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 d

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

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: clang-tools-extra/clang-tidy/bugprone/PosixReturnCh

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

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 doe

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

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 clang/test/Driver/as-w-option

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

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 clang/test/Driver/as-

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

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 clang/tes

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

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] D77168: Add a flag to debug automatic variable initialization

jcai19 added a comment. Just want to follow up and see if anything is missing from the latest patch :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77168/new/ https://reviews.llvm.org/D77168 ___ cfe-c

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

jcai19 added a comment. @jfb Does the current implementation look good? Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77168/new/ https://reviews.llvm.org/D77168 ___ cfe-commits mailing list cfe

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

jcai19 added a comment. In D77168#2069783 , @MaskRay wrote: > Do we have a mechanism bisecting pragmas? If yes, we can let that tool add > `#pragma clang attribute push([[clang::uninitialized]], apply_to = variable)` Not that I am aware of unfortunately

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

jcai19 added a comment. > Right, support needs to be added, and I was wondering if you'd want to do > this because it seems like a tool that would help you out. Sorry for the delay. Yes the biggest advantage of this option would be to make automating bisection much easier. Or are you suggesting

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

jcai19 added a comment. In D77168#2073736 , @jfb wrote: > Can you add a test for the diagnostic firing after the correct number of > initializations? This should include a few types of auto-init, including VLAs. Thank you for the comments! It turned out

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

jcai19 added a comment. > If we have a mechanism bisecting pragmas, this option will not be needed. Yes we would have not had to start to work on this patch if such mechanism had been implemented. We can always migrate to that approach if said mechanism is implemented in the future. I think it

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

jcai19 updated this revision to Diff 269103. jcai19 marked 4 inline comments as done. jcai19 added a comment. Cover VLAs. 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/D

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

jcai19 updated this revision to Diff 269301. jcai19 added a comment. Fix typos. 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/DiagnosticDriverKinds.td clang/include/cl

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

jcai19 marked 2 inline comments as done. jcai19 added a comment. In D77168#2080296 , @jfb wrote: > Two minor corrections, looks good otherwise. Thank you for all the comments and help. I have fixed the issues. Repository: rG LLVM Github Monorepo CHA

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG4db2b7024868: Add a flag to debug automatic variable initialization (authored by jcai19). Changed prior to commit: https://reviews.llvm.org/D77168?vs=269301&id=269324#toc Repository: rG LLVM Github M

[PATCH] D83828: [clang][Driver] update the default location for clang crash diagnostics.

jcai19 created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. jcai19 planned changes to this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D83828 Files: clang/lib/Driver/Driver.cpp clang/test/Driver/crash-report-header.h Ind

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

jcai19 added a comment. > I'd also like to see the pragma attribute approach, as well as byte-pattern > variability as I described. I don't think auto-narrowing is the only approach > we should push people towards. Thank you for all the feedback. I agree, the pragma attribute and the byte-pat

[PATCH] D78693: Make "#pragma clang attribute" support uninitialized attribute.

jcai19 created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When using -ftrivial-auto-var-init=* options to initiate automatic variables in a file, to disable initialization on some variables, currently we have to manually annotate the variables with uninit

[PATCH] D78693: Make "#pragma clang attribute" support uninitialized attribute.

jcai19 added a comment. Thank you all for the comments. I just realized I forgot to update clang documentation. Added that too. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78693/new/ https://reviews.llvm.org/D78693 ___

[PATCH] D78693: Make "#pragma clang attribute" support uninitialized attribute.

jcai19 updated this revision to Diff 259652. jcai19 added a comment. Update test cases and documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78693/new/ https://reviews.llvm.org/D78693 Files: clang/docs/LanguageExtensions.rst clang/i

[PATCH] D78693: Make "#pragma clang attribute" support uninitialized attribute.

This revision was automatically updated to reflect the committed changes. Closed by commit rGfcbc613ad8b7: Make "#pragma clang attribute" support uninitialized attribute. (authored by jcai19). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78693/new/

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

jcai19 updated this revision to Diff 260788. jcai19 added a comment. Update baesd on comments. 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 clang/incl

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

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 s

  1   2   >