from:"Voss, Matthew via cfe\-commits"

RE: [clang] 1029747 - [Sema] Fix null pointer dereference handleAlwaysInlineAttr.

2023-03-16 Thread Voss, Matthew via cfe-commits

LG. Thanks!

> -Original Message-
> From: Craig Topper 
> Sent: Thursday, March 16, 2023 8:01 PM
> To: Voss, Matthew 
> Cc: Craig Topper ; cfe-commits@lists.llvm.org
> Subject: Re: [clang] 1029747 - [Sema] Fix null pointer dereference
> handleAlwaysInlineAttr.
> 
> Thanks for the heads up. Looks like we have a different default C++ standard
> enabled than what the tests got in my local testing. I’ll revert and make a 
> fix.
> 
> > On Mar 16, 2023, at 7:53 PM, Voss, Matthew 
> wrote:
> >
> > Hi Craig,
> >
> > There are some failures on the PS5 buildbot after this change. Could you 
> > take a
> look?
> >
> > INVALID URI REMOVED
> >
> 16/builds/18474__;Iw!!JmoZiZGBv3RvKRSx!8a7C6K5aa0CE1QTOIygEPamFI5WL
> KVq
> > Q-REw0n6KCF54tqKP7df8KtY_duKszo3lif-L572623pQ5nsE68Mw6YhD$
> >
> > Thanks,
> > Matt
> >
> >> -Original Message-
> >> From: cfe-commits  On Behalf Of
> >> Craig Topper via cfe-commits
> >> Sent: Thursday, March 16, 2023 5:52 PM
> >> To: cfe-commits@lists.llvm.org
> >> Subject: [clang] 1029747 - [Sema] Fix null pointer dereference
> >> handleAlwaysInlineAttr.
> >>
> >>
> >> Author: Craig Topper
> >> Date: 2023-03-16T17:49:34-07:00
> >> New Revision: 10297470e953f4f3968c54c851c8af82b07af00b
> >>
> >> URL: INVALID URI REMOVED
> >>
> project/commit/10297470e953f4f3968c54c851c8af82b07af00b__;!!JmoZiZGBv
> >> 3RvKRSx!5T2bYoGu6_sqxRYd-ZFC--KklLgMVa3mP6dp-
> >> DaM426lCB_1eQvZAEGwE8Pe2BFekx30eRBU7KU4wdK2-3ln_WfRflk$
> >> DIFF: INVALID URI REMOVED
> >>
> project/commit/10297470e953f4f3968c54c851c8af82b07af00b.diff__;!!JmoZ
> >> iZ
> >> GBv3RvKRSx!5T2bYoGu6_sqxRYd-ZFC--KklLgMVa3mP6dp-
> >> DaM426lCB_1eQvZAEGwE8Pe2BFekx30eRBU7KU4wdK2-3lnHdJlBwk$
> >>
> >> LOG: [Sema] Fix null pointer dereference handleAlwaysInlineAttr.
> >>
> >> It's possible for `getCalleeDecl()` to return a null pointer.
> >>
> >> This was encountered by a user of our downstream compiler.
> >>
> >> The case involved a DependentScopeDeclRefExpr.
> >>
> >> Since this seems to only be for a warning diagnostic, I skipped the
> >> diagnostic check if it returned null. But mabye there's a different way to 
> >> fix
> this.
> >>
> >> Reviewed By: erichkeane
> >>
> >> Differential Revision:
> >> https://reviews.llvm.org/D146089
> >> iZGBv3RvKRSx!8a7C6K5aa0CE1QTOIygEPamFI5WLKVqQ-
> REw0n6KCF54tqKP7df8KtY_
> >> duKszo3lif-L572623pQ5nsE6xcR7geM$
> >> 3RvKRSx!5T2bYoGu6_sqxRYd-ZFC--KklLgMVa3mP6dp-
> >> DaM426lCB_1eQvZAEGwE8Pe2BFekx30eRBU7KU4wdK2-3lnaIroE7Q$
> >>
> >> Added:
> >>
> >>
> >> Modified:
> >>clang/lib/Sema/SemaStmtAttr.cpp
> >>clang/test/Sema/attr-alwaysinline.cpp
> >>clang/test/Sema/attr-noinline.cpp
> >>
> >> Removed:
> >>
> >>
> >>
> >>
> #
> >> ###
> >> diff  --git a/clang/lib/Sema/SemaStmtAttr.cpp
> >> b/clang/lib/Sema/SemaStmtAttr.cpp index 6d443837a4c5..eeef85373ccb
> >> 100644
> >> --- a/clang/lib/Sema/SemaStmtAttr.cpp
> >> +++ b/clang/lib/Sema/SemaStmtAttr.cpp
> >> @@ -233,7 +233,8 @@ static Attr *handleNoInlineAttr(Sema &S, Stmt
> >> *St, const ParsedAttr &A,
> >>
> >>   for (const auto *CallExpr : CEF.getCallExprs()) {
> >> const Decl *Decl = CallExpr->getCalleeDecl();
> >> -if (Decl->hasAttr() || Decl->hasAttr())
> >> +if (Decl &&
> >> +(Decl->hasAttr() ||
> >> + Decl->hasAttr()))
> >>   S.Diag(St->getBeginLoc(),
> diag::warn_function_stmt_attribute_precedence)
> >>   << A << (Decl->hasAttr() ? 0 : 1);
> >>   }
> >> @@ -259,7 +260,7 @@ static Attr *handleAlwaysInlineAttr(Sema &S, Stmt
> >> *St, const ParsedAttr &A,
> >>
> >>   for (const auto *CallExpr : CEF.getCallExprs()) {
> >> const Decl *Decl = CallExpr->getCalleeDecl();
> >> -if (Decl->hasAttr() || Decl->hasAttr())
> >> +if (Decl && (Decl->hasAttr() ||
> >> + Decl->hasAttr()))
> >>   S.Diag(St->getBeginLoc(),
> diag::warn_function_stmt_attribute_precedence)
> >>   << A << (Decl->hasAttr() ? 2 : 1);
> >>   }
> >>
> >> diff  --git a/clang/test/Sema/attr-alwaysinline.cpp
> >> b/clang/test/Sema/attr- alwaysinline.cpp index
> >> 6b8e8f215a4b..213d70407f48 100644
> >> --- a/clang/test/Sema/attr-alwaysinline.cpp
> >> +++ b/clang/test/Sema/attr-alwaysinline.cpp
> >> @@ -25,3 +25,22 @@ void foo() {
> >> }
> >>
> >> [[clang::always_inline]] static int i = bar(); // expected-warning
> {{'always_inline'
> >> attribute only applies to functions and statements}}
> >> +
> >> +// This used to crash the compiler.
> >> +template
> >> +int foo(int x) {
> >> +if constexpr (D > 1)
> >> +[[clang::always_inline]] return foo(x + 1);
> >> +else
> >> +return x;
> >> +}
> >> +
> >> +// FIXME: This should warn that always_inline statement attribute
> >> +has higher // precedence than the noinline function attribute.
> >> +template [[gnu::noinline]]
> >> +int bar(int x) {
> >> +if constexpr (D > 1)
> >> +[[clang::always_inline]] return bar(x + 1);
> >> +else
> >> +return x;
> >> +}
> >>
> >> diff  --gi

RE: [clang] 1029747 - [Sema] Fix null pointer dereference handleAlwaysInlineAttr.

2023-03-16 Thread Voss, Matthew via cfe-commits

Hi Craig,

There are some failures on the PS5 buildbot after this change. Could you take a 
look?

https://lab.llvm.org/buildbot/#/builders/216/builds/18474

Thanks,
Matt 

> -Original Message-
> From: cfe-commits  On Behalf Of Craig
> Topper via cfe-commits
> Sent: Thursday, March 16, 2023 5:52 PM
> To: cfe-commits@lists.llvm.org
> Subject: [clang] 1029747 - [Sema] Fix null pointer dereference
> handleAlwaysInlineAttr.
> 
> 
> Author: Craig Topper
> Date: 2023-03-16T17:49:34-07:00
> New Revision: 10297470e953f4f3968c54c851c8af82b07af00b
> 
> URL: INVALID URI REMOVED
> project/commit/10297470e953f4f3968c54c851c8af82b07af00b__;!!JmoZiZGBv
> 3RvKRSx!5T2bYoGu6_sqxRYd-ZFC--KklLgMVa3mP6dp-
> DaM426lCB_1eQvZAEGwE8Pe2BFekx30eRBU7KU4wdK2-3ln_WfRflk$
> DIFF: INVALID URI REMOVED
> project/commit/10297470e953f4f3968c54c851c8af82b07af00b.diff__;!!JmoZiZ
> GBv3RvKRSx!5T2bYoGu6_sqxRYd-ZFC--KklLgMVa3mP6dp-
> DaM426lCB_1eQvZAEGwE8Pe2BFekx30eRBU7KU4wdK2-3lnHdJlBwk$
> 
> LOG: [Sema] Fix null pointer dereference handleAlwaysInlineAttr.
> 
> It's possible for `getCalleeDecl()` to return a null pointer.
> 
> This was encountered by a user of our downstream compiler.
> 
> The case involved a DependentScopeDeclRefExpr.
> 
> Since this seems to only be for a warning diagnostic, I skipped the diagnostic
> check if it returned null. But mabye there's a different way to fix this.
> 
> Reviewed By: erichkeane
> 
> Differential Revision:
> https://reviews.llvm.org/D146089
> 3RvKRSx!5T2bYoGu6_sqxRYd-ZFC--KklLgMVa3mP6dp-
> DaM426lCB_1eQvZAEGwE8Pe2BFekx30eRBU7KU4wdK2-3lnaIroE7Q$
> 
> Added:
> 
> 
> Modified:
> clang/lib/Sema/SemaStmtAttr.cpp
> clang/test/Sema/attr-alwaysinline.cpp
> clang/test/Sema/attr-noinline.cpp
> 
> Removed:
> 
> 
> 
> #
> ###
> diff  --git a/clang/lib/Sema/SemaStmtAttr.cpp
> b/clang/lib/Sema/SemaStmtAttr.cpp index 6d443837a4c5..eeef85373ccb
> 100644
> --- a/clang/lib/Sema/SemaStmtAttr.cpp
> +++ b/clang/lib/Sema/SemaStmtAttr.cpp
> @@ -233,7 +233,8 @@ static Attr *handleNoInlineAttr(Sema &S, Stmt *St,
> const ParsedAttr &A,
> 
>for (const auto *CallExpr : CEF.getCallExprs()) {
>  const Decl *Decl = CallExpr->getCalleeDecl();
> -if (Decl->hasAttr() || Decl->hasAttr())
> +if (Decl &&
> +(Decl->hasAttr() ||
> + Decl->hasAttr()))
>S.Diag(St->getBeginLoc(), 
> diag::warn_function_stmt_attribute_precedence)
><< A << (Decl->hasAttr() ? 0 : 1);
>}
> @@ -259,7 +260,7 @@ static Attr *handleAlwaysInlineAttr(Sema &S, Stmt *St,
> const ParsedAttr &A,
> 
>for (const auto *CallExpr : CEF.getCallExprs()) {
>  const Decl *Decl = CallExpr->getCalleeDecl();
> -if (Decl->hasAttr() || Decl->hasAttr())
> +if (Decl && (Decl->hasAttr() ||
> + Decl->hasAttr()))
>S.Diag(St->getBeginLoc(), 
> diag::warn_function_stmt_attribute_precedence)
><< A << (Decl->hasAttr() ? 2 : 1);
>}
> 
> diff  --git a/clang/test/Sema/attr-alwaysinline.cpp b/clang/test/Sema/attr-
> alwaysinline.cpp
> index 6b8e8f215a4b..213d70407f48 100644
> --- a/clang/test/Sema/attr-alwaysinline.cpp
> +++ b/clang/test/Sema/attr-alwaysinline.cpp
> @@ -25,3 +25,22 @@ void foo() {
>  }
> 
>  [[clang::always_inline]] static int i = bar(); // expected-warning 
> {{'always_inline'
> attribute only applies to functions and statements}}
> +
> +// This used to crash the compiler.
> +template
> +int foo(int x) {
> +if constexpr (D > 1)
> +[[clang::always_inline]] return foo(x + 1);
> +else
> +return x;
> +}
> +
> +// FIXME: This should warn that always_inline statement attribute has
> +higher // precedence than the noinline function attribute.
> +template [[gnu::noinline]]
> +int bar(int x) {
> +if constexpr (D > 1)
> +[[clang::always_inline]] return bar(x + 1);
> +else
> +return x;
> +}
> 
> diff  --git a/clang/test/Sema/attr-noinline.cpp b/clang/test/Sema/attr-
> noinline.cpp
> index d35782f11adb..a62ca1debcc5 100644
> --- a/clang/test/Sema/attr-noinline.cpp
> +++ b/clang/test/Sema/attr-noinline.cpp
> @@ -25,3 +25,22 @@ void foo() {
>  }
> 
>  [[clang::noinline]] static int i = bar(); // expected-warning {{'noinline' 
> attribute
> only applies to functions and statements}}
> +
> +// This used to crash the compiler.
> +template
> +int foo(int x) {
> +if constexpr (D > 1)
> +[[clang::noinline]] return foo(x + 1);
> +else
> +return x;
> +}
> +
> +// FIXME: This should warn that noinline statement attribute has higher
> +// precedence than the always_inline function attribute.
> +template [[clang::always_inline]] int bar(int x) {
> +if constexpr (D > 1)
> +[[clang::noinline]] return bar(x + 1);
> +else
> +return x;
> +}
> 
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> INVALID URI REMOVED
> commits__;!!JmoZiZGBv3RvKRSx!5T2bYoGu6_sqxRYd-ZFC--KklLgMVa3mP

RE: [clang] 589ce5f - [DebugInfo] Move constructor homing case in shouldOmitDefinition.

2020-08-31 Thread Voss, Matthew via cfe-commits

Hi David and Amy,

This change works for PS4. Build + test passes.

Thanks,
Matthew

From: David Blaikie 
Sent: Monday, August 31, 2020 1:22 PM
To: Voss, Matthew 
Cc: Amy Huang ; cfe-commits@lists.llvm.org
Subject: Re: [clang] 589ce5f - [DebugInfo] Move constructor homing case in 
shouldOmitDefinition.



On Mon, Aug 24, 2020 at 9:32 PM Voss, Matthew via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Hi Amy,

Looks like there's some test failures on the PS4 Linux bot as a result of this 
commit. Could you take a look? If the failure persists for a while, I may need 
to revert this to unclog the bots and our internal CI.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/73945

Was this issue resolved? The patch was reverted and an update has been posted 
(& approved by me (though looks like it hasn't been recommitted yet)) - any 
chance Amy/Matthew/etc could check there's a good chance this issue is fixed by 
that update/won't regress on the recommit.



Thanks,
Matthew

> -Original Message-
> From: cfe-commits 
> mailto:cfe-commits-boun...@lists.llvm.org>>
>  On Behalf Of Amy
> Huang via cfe-commits
> Sent: Monday, August 24, 2020 8:18 PM
> To: cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>
> Subject: [clang] 589ce5f - [DebugInfo] Move constructor homing case in
> shouldOmitDefinition.
>
>
> Author: Amy Huang
> Date: 2020-08-24T20:17:59-07:00
> New Revision: 589ce5f7050dd83fd3f7dbc182ea0fb051ece994
>
> URL: https://github.com/llvm/llvm-
> project/commit/589ce5f7050dd83fd3f7dbc182ea0fb051ece994
> DIFF: https://github.com/llvm/llvm-
> project/commit/589ce5f7050dd83fd3f7dbc182ea0fb051ece994.diff
>
> LOG: [DebugInfo] Move constructor homing case in shouldOmitDefinition.
>
> For some reason the ctor homing case was before the template
> specialization case, and could have returned false too early.
> I moved the code out into a separate function to avoid this.
>
> Also added a run line to the template specialization test. I guess all the
> -debug-info-kind=limited tests should still pass with =constructor, but
> it's probably unnecessary to test for all of those.
>
> Differential Revision: https://reviews.llvm.org/D86491
>
> Added:
>
>
> Modified:
> clang/lib/CodeGen/CGDebugInfo.cpp
> clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
>
> Removed:
>
>
>
> ##
> ##
> diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp
> b/clang/lib/CodeGen/CGDebugInfo.cpp
> index e3442ecd4bd5..c2929d027a1b 100644
> --- a/clang/lib/CodeGen/CGDebugInfo.cpp
> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
> @@ -2260,6 +2260,25 @@ static bool
> hasExplicitMemberDefinition(CXXRecordDecl::method_iterator I,
>return false;
>  }
>
> +static bool canUseCtorHoming(const CXXRecordDecl *RD) {
> +  // Constructor homing can be used for classes that have at least one
> +  // constructor and have no trivial or constexpr constructors.
> +  // Skip this optimization if the class or any of its methods are
> +marked
> +  // dllimport.
> +  if (RD->isLambda() || RD->hasConstexprNonCopyMoveConstructor() ||
> +  isClassOrMethodDLLImport(RD))
> +return false;
> +
> +  if (RD->ctors().empty())
> +return false;
> +
> +  for (const auto *Ctor : RD->ctors())
> +if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
> +  return false;
> +
> +  return true;
> +}
> +
>  static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
>   bool DebugTypeExtRefs, const RecordDecl
> *RD,
>   const LangOptions &LangOpts) { @@ -
> 2294,23 +2313,6 @@ static bool
> shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
>!isClassOrMethodDLLImport(CXXDecl))
>  return true;
>
> -  // In constructor debug mode, only emit debug info for a class when its
> -  // constructor is emitted. Skip this optimization if the class or any
> of
> -  // its methods are marked dllimport.
> -  //
> -  // This applies to classes that don't have any trivial constructors and
> have
> -  // at least one constructor.
> -  if (DebugKind == codegenoptions::DebugInfoConstructor &&
> -  !CXXDecl->isLambda() && !CXXDecl-
> >hasConstexprNonCopyMoveConstructor() &&
> -  !isClassOrMethodDLLImport(CXXDecl)) {
> -if (CXXDecl->ctors().empty())
> -  return false;
> -for (const auto *Ctor : CXXDecl->ctors())
> -  if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
> -

RE: [clang] 589ce5f - [DebugInfo] Move constructor homing case in shouldOmitDefinition.

2020-08-24 Thread Voss, Matthew via cfe-commits

Hi Amy,

Looks like there's some test failures on the PS4 Linux bot as a result of this 
commit. Could you take a look? If the failure persists for a while, I may need 
to revert this to unclog the bots and our internal CI.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/73945

Thanks,
Matthew

> -Original Message-
> From: cfe-commits  On Behalf Of Amy
> Huang via cfe-commits
> Sent: Monday, August 24, 2020 8:18 PM
> To: cfe-commits@lists.llvm.org
> Subject: [clang] 589ce5f - [DebugInfo] Move constructor homing case in
> shouldOmitDefinition.
> 
> 
> Author: Amy Huang
> Date: 2020-08-24T20:17:59-07:00
> New Revision: 589ce5f7050dd83fd3f7dbc182ea0fb051ece994
> 
> URL: https://github.com/llvm/llvm-
> project/commit/589ce5f7050dd83fd3f7dbc182ea0fb051ece994
> DIFF: https://github.com/llvm/llvm-
> project/commit/589ce5f7050dd83fd3f7dbc182ea0fb051ece994.diff
> 
> LOG: [DebugInfo] Move constructor homing case in shouldOmitDefinition.
> 
> For some reason the ctor homing case was before the template
> specialization case, and could have returned false too early.
> I moved the code out into a separate function to avoid this.
> 
> Also added a run line to the template specialization test. I guess all the
> -debug-info-kind=limited tests should still pass with =constructor, but
> it's probably unnecessary to test for all of those.
> 
> Differential Revision: https://reviews.llvm.org/D86491
> 
> Added:
> 
> 
> Modified:
> clang/lib/CodeGen/CGDebugInfo.cpp
> clang/test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
> 
> Removed:
> 
> 
> 
> ##
> ##
> diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp
> b/clang/lib/CodeGen/CGDebugInfo.cpp
> index e3442ecd4bd5..c2929d027a1b 100644
> --- a/clang/lib/CodeGen/CGDebugInfo.cpp
> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
> @@ -2260,6 +2260,25 @@ static bool
> hasExplicitMemberDefinition(CXXRecordDecl::method_iterator I,
>return false;
>  }
> 
> +static bool canUseCtorHoming(const CXXRecordDecl *RD) {
> +  // Constructor homing can be used for classes that have at least one
> +  // constructor and have no trivial or constexpr constructors.
> +  // Skip this optimization if the class or any of its methods are
> +marked
> +  // dllimport.
> +  if (RD->isLambda() || RD->hasConstexprNonCopyMoveConstructor() ||
> +  isClassOrMethodDLLImport(RD))
> +return false;
> +
> +  if (RD->ctors().empty())
> +return false;
> +
> +  for (const auto *Ctor : RD->ctors())
> +if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
> +  return false;
> +
> +  return true;
> +}
> +
>  static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
>   bool DebugTypeExtRefs, const RecordDecl
> *RD,
>   const LangOptions &LangOpts) { @@ -
> 2294,23 +2313,6 @@ static bool
> shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
>!isClassOrMethodDLLImport(CXXDecl))
>  return true;
> 
> -  // In constructor debug mode, only emit debug info for a class when its
> -  // constructor is emitted. Skip this optimization if the class or any
> of
> -  // its methods are marked dllimport.
> -  //
> -  // This applies to classes that don't have any trivial constructors and
> have
> -  // at least one constructor.
> -  if (DebugKind == codegenoptions::DebugInfoConstructor &&
> -  !CXXDecl->isLambda() && !CXXDecl-
> >hasConstexprNonCopyMoveConstructor() &&
> -  !isClassOrMethodDLLImport(CXXDecl)) {
> -if (CXXDecl->ctors().empty())
> -  return false;
> -for (const auto *Ctor : CXXDecl->ctors())
> -  if (Ctor->isTrivial() && !Ctor->isCopyOrMoveConstructor())
> -return false;
> -return true;
> -  }
> -
>TemplateSpecializationKind Spec = TSK_Undeclared;
>if (const auto *SD = dyn_cast(RD))
>  Spec = SD->getSpecializationKind(); @@ -2320,6 +2322,12 @@ static
> bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
>CXXDecl->method_end()))
>  return true;
> 
> +  // In constructor homing mode, only emit complete debug info for a
> + class  // when its constructor is emitted.
> +  if ((DebugKind != codegenoptions::DebugInfoConstructor) &&
> +  canUseCtorHoming(CXXDecl))
> +return true;
> +
>return false;
>  }
> 
> 
> diff  --git a/clang/test/CodeGenCXX/debug-info-template-explicit-
> specialization.cpp b/clang/test/CodeGenCXX/debug-info-template-explicit-
> specialization.cpp
> index 4e41c4092bf4..ff0457e94404 100644
> --- a/clang/test/CodeGenCXX/debug-info-template-explicit-
> specialization.cpp
> +++ b/clang/test/CodeGenCXX/debug-info-template-explicit-specialization.
> +++ cpp
> @@ -1,4 +1,7 @@
> -// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_triple -debug-info-
> kind=limited %s -o - | FileCheck %s
> +// RUN: %clang_cc1 -emit-llvm -triple %itanium_abi_trip

Re: [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics

2020-05-19 Thread Voss, Matthew via cfe-commits

Looks good. Thanks!

Matthew

From: Sam McCall 
Sent: Tuesday, May 19, 2020 9:19 AM
To: Voss, Matthew 
Cc: cfe-commits ; Sam McCall 
Subject: Re: [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace 
metrics

Sorry about that Matthew, I think 5bc0c8f009261425a should have fixed this 
(giving up on stringview entirely). It fixed the other windows based bots.

If it's no good, please do revert if you can, I'll check on it shortly too.

On Tue, May 19, 2020, 6:14 PM Voss, Matthew 
mailto:matthew.v...@sony.com>> wrote:
Hi Sam,

This commit's tests are failing to compile on the PS4 Windows bot and our 
internal CI. Could you take a look? Just FYI, I may end up reverting this after 
an hour or so to unclog our CI.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/32433

Thanks,
Matthew

From: cfe-commits 
mailto:cfe-commits-boun...@lists.llvm.org>> 
on behalf of Sam McCall via cfe-commits 
mailto:cfe-commits@lists.llvm.org>>
Sent: Tuesday, May 19, 2020 4:36 AM
To: cfe-commits@lists.llvm.org 
mailto:cfe-commits@lists.llvm.org>>
Subject: [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics


Author: Sam McCall
Date: 2020-05-19T13:35:31+02:00
New Revision: 9b88a190b42a03753b9c49ccea34514cb40ba4ab

URL: 
https://github.com/llvm/llvm-project/commit/9b88a190b42a03753b9c49ccea34514cb40ba4ab
DIFF: 
https://github.com/llvm/llvm-project/commit/9b88a190b42a03753b9c49ccea34514cb40ba4ab.diff

LOG: [clangd] Add CSV export for trace metrics

Summary: Example: 
https://docs.google.com/spreadsheets/d/1VZKGetSUTTDe9p4ooIETmdcwUod1_aE3vgD0E9x7HhI/edit

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, jfb, usaxena95, 
cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D79678

Added:
clang-tools-extra/clangd/test/metrics.test

Modified:
clang-tools-extra/clangd/support/Trace.cpp
clang-tools-extra/clangd/support/Trace.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/support/TraceTests.cpp

Removed:




diff  --git a/clang-tools-extra/clangd/support/Trace.cpp 
b/clang-tools-extra/clangd/support/Trace.cpp
index 6bf4816268e5..10ae461221ee 100644
--- a/clang-tools-extra/clangd/support/Trace.cpp
+++ b/clang-tools-extra/clangd/support/Trace.cpp
@@ -189,6 +189,66 @@ class JSONTracer : public EventTracer {
   const llvm::sys::TimePoint<> Start;
 };

+// We emit CSV as specified in RFC 4180: https://www.ietf.org/rfc/rfc4180.txt.
+// \r\n line endings are used, cells with \r\n," are quoted, quotes are 
doubled.
+class CSVMetricTracer : public EventTracer {
+public:
+  CSVMetricTracer(llvm::raw_ostream &Out) : Out(Out) {
+Start = std::chrono::steady_clock::now();
+
+Out.SetUnbuffered(); // We write each line atomically.
+Out << "Kind,Metric,Label,Value,Timestamp\r\n";
+  }
+
+  void record(const Metric &Metric, double Value,
+  llvm::StringRef Label) override {
+assert(!needsQuote(Metric.Name));
+std::string QuotedLabel;
+if (needsQuote(Label))
+  Label = QuotedLabel = quote(Label);
+uint64_t Micros = std::chrono::duration_cast(
+  std::chrono::steady_clock::now() - Start)
+  .count();
+std::lock_guard Lock(Mu);
+Out << llvm::formatv("{0},{1},{2},{3:e},{4}.{5:6}\r\n",
+ typeName(Metric.Type), Metric.Name, Label, Value,
+ Micros / 100, Micros % 100);
+  }
+
+private:
+  llvm::StringRef typeName(Metric::MetricType T) {
+switch (T) {
+case Metric::Value:
+  return "v";
+case Metric::Counter:
+  return "c";
+case Metric::Distribution:
+  return "d";
+}
+  }
+
+  static bool needsQuote(llvm::StringRef Text) {
+// https://www.ietf.org/rfc/rfc4180.txt section 2.6
+return Text.find_first_of(",\"\r\n") != llvm::StringRef::npos;
+  }
+
+  std::string quote(llvm::StringRef Text) {
+std::string Result = "\"";
+for (char C : Text) {
+  Result.push_back(C);
+  if (C == '"')
+Result.push_back('"');
+}
+Result.push_back('"');
+return Result;
+  }
+
+private:
+  std::mutex Mu;
+  llvm::raw_ostream &Out /*GUARDED_BY(Mu)*/;
+  std::chrono::steady_clock::time_point Start;
+};
+
 Key> JSONTracer::SpanKey;

 EventTracer *T = nullptr;
@@ -206,6 +266,10 @@ std::unique_ptr 
createJSONTracer(llvm::raw_ostream &OS,
   return std::make_unique(OS, Pretty);
 }

+std::unique_ptr createCSVMetricTracer(llvm::raw_ostream &OS) {
+  return std::make_unique(OS);
+}
+
 void log(const llvm::Twine &Message) {
   if (!T)
 return;

diff  --git a/clang-tools-extra/clangd/support/Trace.h 
b/clang-tools-extra/clangd/support/Trace.h
index 90a11bb1feb4..9dc397a84b74 100644
--- a/clang-tools-extra/clangd/support

Re: [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics

2020-05-19 Thread Voss, Matthew via cfe-commits

Hi Sam,

This commit's tests are failing to compile on the PS4 Windows bot and our 
internal CI. Could you take a look? Just FYI, I may end up reverting this after 
an hour or so to unclog our CI.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/32433

Thanks,
Matthew

From: cfe-commits  on behalf of Sam McCall 
via cfe-commits 
Sent: Tuesday, May 19, 2020 4:36 AM
To: cfe-commits@lists.llvm.org 
Subject: [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics


Author: Sam McCall
Date: 2020-05-19T13:35:31+02:00
New Revision: 9b88a190b42a03753b9c49ccea34514cb40ba4ab

URL: 
https://github.com/llvm/llvm-project/commit/9b88a190b42a03753b9c49ccea34514cb40ba4ab
DIFF: 
https://github.com/llvm/llvm-project/commit/9b88a190b42a03753b9c49ccea34514cb40ba4ab.diff

LOG: [clangd] Add CSV export for trace metrics

Summary: Example: 
https://docs.google.com/spreadsheets/d/1VZKGetSUTTDe9p4ooIETmdcwUod1_aE3vgD0E9x7HhI/edit

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, jfb, usaxena95, 
cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D79678

Added:
clang-tools-extra/clangd/test/metrics.test

Modified:
clang-tools-extra/clangd/support/Trace.cpp
clang-tools-extra/clangd/support/Trace.h
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/support/TraceTests.cpp

Removed:




diff  --git a/clang-tools-extra/clangd/support/Trace.cpp 
b/clang-tools-extra/clangd/support/Trace.cpp
index 6bf4816268e5..10ae461221ee 100644
--- a/clang-tools-extra/clangd/support/Trace.cpp
+++ b/clang-tools-extra/clangd/support/Trace.cpp
@@ -189,6 +189,66 @@ class JSONTracer : public EventTracer {
   const llvm::sys::TimePoint<> Start;
 };

+// We emit CSV as specified in RFC 4180: https://www.ietf.org/rfc/rfc4180.txt.
+// \r\n line endings are used, cells with \r\n," are quoted, quotes are 
doubled.
+class CSVMetricTracer : public EventTracer {
+public:
+  CSVMetricTracer(llvm::raw_ostream &Out) : Out(Out) {
+Start = std::chrono::steady_clock::now();
+
+Out.SetUnbuffered(); // We write each line atomically.
+Out << "Kind,Metric,Label,Value,Timestamp\r\n";
+  }
+
+  void record(const Metric &Metric, double Value,
+  llvm::StringRef Label) override {
+assert(!needsQuote(Metric.Name));
+std::string QuotedLabel;
+if (needsQuote(Label))
+  Label = QuotedLabel = quote(Label);
+uint64_t Micros = std::chrono::duration_cast(
+  std::chrono::steady_clock::now() - Start)
+  .count();
+std::lock_guard Lock(Mu);
+Out << llvm::formatv("{0},{1},{2},{3:e},{4}.{5:6}\r\n",
+ typeName(Metric.Type), Metric.Name, Label, Value,
+ Micros / 100, Micros % 100);
+  }
+
+private:
+  llvm::StringRef typeName(Metric::MetricType T) {
+switch (T) {
+case Metric::Value:
+  return "v";
+case Metric::Counter:
+  return "c";
+case Metric::Distribution:
+  return "d";
+}
+  }
+
+  static bool needsQuote(llvm::StringRef Text) {
+// https://www.ietf.org/rfc/rfc4180.txt section 2.6
+return Text.find_first_of(",\"\r\n") != llvm::StringRef::npos;
+  }
+
+  std::string quote(llvm::StringRef Text) {
+std::string Result = "\"";
+for (char C : Text) {
+  Result.push_back(C);
+  if (C == '"')
+Result.push_back('"');
+}
+Result.push_back('"');
+return Result;
+  }
+
+private:
+  std::mutex Mu;
+  llvm::raw_ostream &Out /*GUARDED_BY(Mu)*/;
+  std::chrono::steady_clock::time_point Start;
+};
+
 Key> JSONTracer::SpanKey;

 EventTracer *T = nullptr;
@@ -206,6 +266,10 @@ std::unique_ptr 
createJSONTracer(llvm::raw_ostream &OS,
   return std::make_unique(OS, Pretty);
 }

+std::unique_ptr createCSVMetricTracer(llvm::raw_ostream &OS) {
+  return std::make_unique(OS);
+}
+
 void log(const llvm::Twine &Message) {
   if (!T)
 return;

diff  --git a/clang-tools-extra/clangd/support/Trace.h 
b/clang-tools-extra/clangd/support/Trace.h
index 90a11bb1feb4..9dc397a84b74 100644
--- a/clang-tools-extra/clangd/support/Trace.h
+++ b/clang-tools-extra/clangd/support/Trace.h
@@ -108,11 +108,18 @@ class Session {
 /// Create an instance of EventTracer that produces an output in the Trace 
Event
 /// format supported by Chrome's trace viewer (chrome://tracing).
 ///
+/// FIXME: Metrics are not recorded, some could become counter events.
+///
 /// The format is documented here:
 /// 
https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
 std::unique_ptr createJSONTracer(llvm::raw_ostream &OS,
   bool Pretty = false);

+/// Create an instance of EventTracer that outputs metric measurements as CSV.
+///
+/// Trace spans and instant events are ignored

RE: [clang-tools-extra] ebdb98f - [clang-tidy] Move fuchsia-restrict-system-includes to portability module for general use.

2020-03-10 Thread Voss, Matthew via cfe-commits

Hi Paula,

The tests in this commit are failing on the PS4 Windows bot and our internal 
CI. Could you take a look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast

Thanks,
Matthew

> -Original Message-
> From: cfe-commits  On Behalf Of Paula
> Toth via cfe-commits
> Sent: Tuesday, March 10, 2020 1:34 PM
> To: cfe-commits@lists.llvm.org
> Subject: [clang-tools-extra] ebdb98f - [clang-tidy] Move fuchsia-restrict-
> system-includes to portability module for general use.
> 
> 
> Author: Paula Toth
> Date: 2020-03-10T13:33:06-07:00
> New Revision: ebdb98f254f632b506109b9d20c6e8e19697765f
> 
> URL: https://github.com/llvm/llvm-
> project/commit/ebdb98f254f632b506109b9d20c6e8e19697765f
> DIFF: https://github.com/llvm/llvm-
> project/commit/ebdb98f254f632b506109b9d20c6e8e19697765f.diff
> 
> LOG: [clang-tidy] Move fuchsia-restrict-system-includes to portability
> module for general use.
> 
> Summary:
> Created a general check for restrict-system-includes under portability as
> recommend in the comments under D75332. I also fleshed out the user facing
> documentation to show examples for common use-cases such as allow-list,
> block-list, and wild carding.
> 
> Removed fuchsia's check as per phosek sugguestion.
> 
> Reviewers: aaron.ballman, phosek, alexfh, hokein, njames93
> 
> Reviewed By: phosek
> 
> Subscribers: Eugene.Zelenko, mgorny, xazax.hun, phosek, cfe-commits,
> MaskRay
> 
> Tags: #clang-tools-extra, #clang
> 
> Differential Revision: https://reviews.llvm.org/D75786
> 
> Added:
> clang-tools-extra/clang-
> tidy/portability/RestrictSystemIncludesCheck.cpp
> clang-tools-extra/clang-tidy/portability/RestrictSystemIncludesCheck.h
> clang-tools-extra/docs/clang-tidy/checks/portability-restrict-system-
> includes.rst
> clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-
> restrict-system-includes/a.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-
> restrict-system-includes/system/cstdarg.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-
> restrict-system-includes/system/cstdlib.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-
> restrict-system-includes/system/j.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-
> restrict-system-includes/system/r.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-
> restrict-system-includes/system/s.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-
> restrict-system-includes/system/t.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-
> restrict-system-includes/system/transitive.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/portability-
> restrict-system-includes/transitive2.h
> clang-tools-extra/test/clang-tidy/checkers/portability-restrict-
> system-includes-allow.cpp
> clang-tools-extra/test/clang-tidy/checkers/portability-restrict-
> system-includes-disallow.cpp
> clang-tools-extra/test/clang-tidy/checkers/portability-restrict-
> system-includes-glob.cpp
> clang-tools-extra/test/clang-tidy/checkers/portability-restrict-
> system-includes-transitive.cpp
> 
> Modified:
> clang-tools-extra/clang-tidy/fuchsia/CMakeLists.txt
> clang-tools-extra/clang-tidy/fuchsia/FuchsiaTidyModule.cpp
> clang-tools-extra/clang-tidy/portability/CMakeLists.txt
> clang-tools-extra/clang-tidy/portability/PortabilityTidyModule.cpp
> clang-tools-extra/docs/ReleaseNotes.rst
> clang-tools-extra/docs/clang-tidy/checks/list.rst
> 
> Removed:
> clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.cpp
> clang-tools-extra/clang-tidy/fuchsia/RestrictSystemIncludesCheck.h
> clang-tools-extra/docs/clang-tidy/checks/fuchsia-restrict-system-
> includes.rst
> clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-
> system-includes/a.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-
> system-includes/system/cstdarg.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-
> system-includes/system/cstdlib.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-
> system-includes/system/j.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-
> system-includes/system/r.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-
> system-includes/system/s.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-
> system-includes/system/t.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-
> system-includes/system/transitive.h
> clang-tools-extra/test/clang-tidy/checkers/Inputs/fuchsia-restrict-
> system-includes/transitive2.h
> clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-
> includes-all.cpp
> clang-tools-extra/test/clang-tidy/checkers/fuchsia-restrict-system-
> includes-glob.cpp
> clang-tools-extra/test/clang-tidy/checkers/fuchsia-restri

RE: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a dependency in cc1

2019-11-08 Thread Voss, Matthew via cfe-commits

Thanks! LGTM

> -Original Message-
> From: jkor...@apple.com 
> Sent: Friday, November 8, 2019 3:33 PM
> To: Voss, Matthew 
> Cc: Jan Korous ; cfe-commits@lists.llvm.org;
> jeremy.morse.l...@gmail.com
> Subject: Re: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a
> dependency in cc1
> 
> Yes, sorry, I messed up when relanding the patch. Seems fixed now.
> 
> > On Nov 8, 2019, at 2:24 PM, Voss, Matthew  wrote:
> >
> > Hi Jan,
> >
> > Thanks for looking at this. I'm seeing new errors on the PS4 windows
> bot.
> >
> > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubunt
> > u-fast/builds/57835
> >
> > Thanks,
> > Matthew
> >
> >> -Original Message-
> >> From: jkor...@apple.com 
> >> Sent: Friday, November 8, 2019 2:01 PM
> >> To: Voss, Matthew 
> >> Cc: Jan Korous ; cfe-commits@lists.llvm.org;
> >> jeremy.morse.l...@gmail.com
> >> Subject: Re: [clang] 03b84e4 - [clang] Report sanitizer blacklist as
> >> a dependency in cc1
> >>
> >> Hi Matthew,
> >>
> >> You were absolutely right - my bad!
> >>
> >> I relanded the patches plus a fix.
> >>
> >> 555c6be041d [clang] Fix -fsanitize-system-blacklist processing in cc1
> >>
> >> Thanks,
> >>
> >> Jan
> >>
> >>> On Nov 8, 2019, at 11:00 AM, Voss, Matthew 
> >> wrote:
> >>>
> >>> Hi Jan,
> >>>
>  Are you sure it is this commit?
> >>> I narrowed it down to your commit on my local machine. Let me know
> >>> if it
> >> doesn't repro on your end, though.
> >>>
> >>> Thanks,
> >>> Matthew
> >>>
> >>>
>  -Original Message-
>  From: jkor...@apple.com 
>  Sent: Friday, November 8, 2019 10:55 AM
>  To: Voss, Matthew 
>  Cc: Jan Korous ; cfe-commits@lists.llvm.org;
>  jeremy.morse.l...@gmail.com
>  Subject: Re: [clang] 03b84e4 - [clang] Report sanitizer blacklist
>  as a dependency in cc1
> 
>  Hi Matthew,
> 
>  Are you sure it is this commit? It is definitely possible yet
>  sounds a bit unlikely that my patch would cause linker errors.
> 
>  Anyway, I am going to reproduce on a linux box.
> 
>  Thanks.
> 
>  Jan
> 
> > On Nov 7, 2019, at 4:50 PM, Voss, Matthew 
> >> wrote:
> >
> > Hi Jan,
> >
> > It looks like this commit is causing DFSAN failures on the
> > sanitizer
>  bots and our internal CI. Could you take a look?
> >
> > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/24
> > 31 2/ steps/64-bit%20check-dfsan/logs/stdio
> >
> > Thanks,
> > Matthew
> >
> >> -Original Message-
> >> From: cfe-commits  On Behalf
> >> Of Jan Korous via cfe-commits
> >> Sent: Thursday, November 7, 2019 2:07 PM
> >> To: cfe-commits@lists.llvm.org
> >> Subject: [clang] 03b84e4 - [clang] Report sanitizer blacklist as
> >> a dependency in cc1
> >>
> >>
> >> Author: Jan Korous
> >> Date: 2019-11-07T14:06:43-08:00
> >> New Revision: 03b84e4f6d0e1c04f22d69cc445f36e1f713beb4
> >>
> >> URL: https://github.com/llvm/llvm-
> >> project/commit/03b84e4f6d0e1c04f22d69cc445f36e1f713beb4
> >> DIFF: https://github.com/llvm/llvm-
> >> project/commit/03b84e4f6d0e1c04f22d69cc445f36e1f713beb4.diff
> >>
> >> LOG: [clang] Report sanitizer blacklist as a dependency in cc1
> >>
> >> Previously these were reported from the driver which blocked
> >> clang-scan- deps from getting the full set of dependencies from
> >> cc1
>  commands.
> >>
> >> Also the default sanitizer blacklist that is added in driver was
> >> never reported as a dependency. I introduced
> >> -fsanitize-system-blacklist cc1 option to keep track of which
> >> blacklists were user-specified and which were added by driver and
> >> clang -MD now also reports system blacklists as dependencies.
> >>
> >> Differential Revision: https://reviews.llvm.org/D69290
> >>
> >> Added:
> >>
> >>
> >> Modified:
> >>  clang/include/clang/Driver/Options.td
> >>  clang/include/clang/Driver/SanitizerArgs.h
> >>  clang/lib/Driver/SanitizerArgs.cpp
> >>  clang/lib/Frontend/CompilerInvocation.cpp
> >>  clang/test/Driver/fsanitize-blacklist.c
> >>  clang/test/Frontend/dependency-gen.c
> >>
> >> Removed:
> >>
> >>
> >>
> >> #
> >> ##
> >> ##
> >> #
> >> ##
> >> diff  --git a/clang/include/clang/Driver/Options.td
> >> b/clang/include/clang/Driver/Options.td
> >> index dcd2976a97f2..c2e30a16b8da 100644
> >> --- a/clang/include/clang/Driver/Options.td
> >> +++ b/clang/include/clang/Driver/Options.td
> >> @@ -979,6 +979,9 @@ def fno_sanitize_EQ : CommaJoined<["-"],
> >> "fno- sanitize=">, Group,  def fsanitize_blacklist :
> >> Joined<["- "], "fsanitize-blacklist=">,
> >> Group,
> >> HelpText<"Path to blackl

RE: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a dependency in cc1

2019-11-08 Thread Voss, Matthew via cfe-commits

Hi Jan,

Thanks for looking at this. I'm seeing new errors on the PS4 windows bot.

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/57835

Thanks,
Matthew

> -Original Message-
> From: jkor...@apple.com 
> Sent: Friday, November 8, 2019 2:01 PM
> To: Voss, Matthew 
> Cc: Jan Korous ; cfe-commits@lists.llvm.org;
> jeremy.morse.l...@gmail.com
> Subject: Re: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a
> dependency in cc1
> 
> Hi Matthew,
> 
> You were absolutely right - my bad!
> 
> I relanded the patches plus a fix.
> 
> 555c6be041d [clang] Fix -fsanitize-system-blacklist processing in cc1
> 
> Thanks,
> 
> Jan
> 
> > On Nov 8, 2019, at 11:00 AM, Voss, Matthew 
> wrote:
> >
> > Hi Jan,
> >
> >> Are you sure it is this commit?
> > I narrowed it down to your commit on my local machine. Let me know if it
> doesn't repro on your end, though.
> >
> > Thanks,
> > Matthew
> >
> >
> >> -Original Message-
> >> From: jkor...@apple.com 
> >> Sent: Friday, November 8, 2019 10:55 AM
> >> To: Voss, Matthew 
> >> Cc: Jan Korous ; cfe-commits@lists.llvm.org;
> >> jeremy.morse.l...@gmail.com
> >> Subject: Re: [clang] 03b84e4 - [clang] Report sanitizer blacklist as
> >> a dependency in cc1
> >>
> >> Hi Matthew,
> >>
> >> Are you sure it is this commit? It is definitely possible yet sounds
> >> a bit unlikely that my patch would cause linker errors.
> >>
> >> Anyway, I am going to reproduce on a linux box.
> >>
> >> Thanks.
> >>
> >> Jan
> >>
> >>> On Nov 7, 2019, at 4:50 PM, Voss, Matthew 
> wrote:
> >>>
> >>> Hi Jan,
> >>>
> >>> It looks like this commit is causing DFSAN failures on the sanitizer
> >> bots and our internal CI. Could you take a look?
> >>>
> >>> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/2431
> >>> 2/ steps/64-bit%20check-dfsan/logs/stdio
> >>>
> >>> Thanks,
> >>> Matthew
> >>>
>  -Original Message-
>  From: cfe-commits  On Behalf Of
>  Jan Korous via cfe-commits
>  Sent: Thursday, November 7, 2019 2:07 PM
>  To: cfe-commits@lists.llvm.org
>  Subject: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a
>  dependency in cc1
> 
> 
>  Author: Jan Korous
>  Date: 2019-11-07T14:06:43-08:00
>  New Revision: 03b84e4f6d0e1c04f22d69cc445f36e1f713beb4
> 
>  URL: https://github.com/llvm/llvm-
>  project/commit/03b84e4f6d0e1c04f22d69cc445f36e1f713beb4
>  DIFF: https://github.com/llvm/llvm-
>  project/commit/03b84e4f6d0e1c04f22d69cc445f36e1f713beb4.diff
> 
>  LOG: [clang] Report sanitizer blacklist as a dependency in cc1
> 
>  Previously these were reported from the driver which blocked
>  clang-scan- deps from getting the full set of dependencies from cc1
> >> commands.
> 
>  Also the default sanitizer blacklist that is added in driver was
>  never reported as a dependency. I introduced
>  -fsanitize-system-blacklist cc1 option to keep track of which
>  blacklists were user-specified and which were added by driver and
>  clang -MD now also reports system blacklists as dependencies.
> 
>  Differential Revision: https://reviews.llvm.org/D69290
> 
>  Added:
> 
> 
>  Modified:
>    clang/include/clang/Driver/Options.td
>    clang/include/clang/Driver/SanitizerArgs.h
>    clang/lib/Driver/SanitizerArgs.cpp
>    clang/lib/Frontend/CompilerInvocation.cpp
>    clang/test/Driver/fsanitize-blacklist.c
>    clang/test/Frontend/dependency-gen.c
> 
>  Removed:
> 
> 
> 
>  ###
>  ##
>  #
>  ##
>  diff  --git a/clang/include/clang/Driver/Options.td
>  b/clang/include/clang/Driver/Options.td
>  index dcd2976a97f2..c2e30a16b8da 100644
>  --- a/clang/include/clang/Driver/Options.td
>  +++ b/clang/include/clang/Driver/Options.td
>  @@ -979,6 +979,9 @@ def fno_sanitize_EQ : CommaJoined<["-"], "fno-
>  sanitize=">, Group,  def fsanitize_blacklist :
>  Joined<["- "], "fsanitize-blacklist=">,
>   Group,
>   HelpText<"Path to blacklist file for
>  sanitizers">;
>  +def fsanitize_system_blacklist : Joined<["-"],
>  +"fsanitize-system-blacklist=">,
>  +  HelpText<"Path to system blacklist file for sanitizers">,
>  +  Flags<[CC1Option]>;
>  def fno_sanitize_blacklist : Flag<["-"], "fno-sanitize-blacklist">,
>  Group,
>  HelpText<"Don't use blacklist file for
>  sanitizers">;
> 
>  diff  --git a/clang/include/clang/Driver/SanitizerArgs.h
>  b/clang/include/clang/Driver/SanitizerArgs.h
>  index c37499e0f201..0aebf8cb225d 100644
>  --- a/clang/include/clang/Driver/SanitizerArgs.h
>  +++ b/clang/include/clang/Driver/SanitizerArgs.h
>  @@ -25,8 +25,8 @@ class SanitizerArgs {  Saniti

RE: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a dependency in cc1

2019-11-08 Thread Voss, Matthew via cfe-commits

Hi Jan,

> Are you sure it is this commit?
I narrowed it down to your commit on my local machine. Let me know if it 
doesn't repro on your end, though.

Thanks,
Matthew


> -Original Message-
> From: jkor...@apple.com 
> Sent: Friday, November 8, 2019 10:55 AM
> To: Voss, Matthew 
> Cc: Jan Korous ; cfe-commits@lists.llvm.org;
> jeremy.morse.l...@gmail.com
> Subject: Re: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a
> dependency in cc1
> 
> Hi Matthew,
> 
> Are you sure it is this commit? It is definitely possible yet sounds a bit
> unlikely that my patch would cause linker errors.
> 
> Anyway, I am going to reproduce on a linux box.
> 
> Thanks.
> 
> Jan
> 
> > On Nov 7, 2019, at 4:50 PM, Voss, Matthew  wrote:
> >
> > Hi Jan,
> >
> > It looks like this commit is causing DFSAN failures on the sanitizer
> bots and our internal CI. Could you take a look?
> >
> > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/24312/
> > steps/64-bit%20check-dfsan/logs/stdio
> >
> > Thanks,
> > Matthew
> >
> >> -Original Message-
> >> From: cfe-commits  On Behalf Of
> >> Jan Korous via cfe-commits
> >> Sent: Thursday, November 7, 2019 2:07 PM
> >> To: cfe-commits@lists.llvm.org
> >> Subject: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a
> >> dependency in cc1
> >>
> >>
> >> Author: Jan Korous
> >> Date: 2019-11-07T14:06:43-08:00
> >> New Revision: 03b84e4f6d0e1c04f22d69cc445f36e1f713beb4
> >>
> >> URL: https://github.com/llvm/llvm-
> >> project/commit/03b84e4f6d0e1c04f22d69cc445f36e1f713beb4
> >> DIFF: https://github.com/llvm/llvm-
> >> project/commit/03b84e4f6d0e1c04f22d69cc445f36e1f713beb4.diff
> >>
> >> LOG: [clang] Report sanitizer blacklist as a dependency in cc1
> >>
> >> Previously these were reported from the driver which blocked
> >> clang-scan- deps from getting the full set of dependencies from cc1
> commands.
> >>
> >> Also the default sanitizer blacklist that is added in driver was
> >> never reported as a dependency. I introduced
> >> -fsanitize-system-blacklist cc1 option to keep track of which
> >> blacklists were user-specified and which were added by driver and
> >> clang -MD now also reports system blacklists as dependencies.
> >>
> >> Differential Revision: https://reviews.llvm.org/D69290
> >>
> >> Added:
> >>
> >>
> >> Modified:
> >>clang/include/clang/Driver/Options.td
> >>clang/include/clang/Driver/SanitizerArgs.h
> >>clang/lib/Driver/SanitizerArgs.cpp
> >>clang/lib/Frontend/CompilerInvocation.cpp
> >>clang/test/Driver/fsanitize-blacklist.c
> >>clang/test/Frontend/dependency-gen.c
> >>
> >> Removed:
> >>
> >>
> >>
> >> #
> >> #
> >> ##
> >> diff  --git a/clang/include/clang/Driver/Options.td
> >> b/clang/include/clang/Driver/Options.td
> >> index dcd2976a97f2..c2e30a16b8da 100644
> >> --- a/clang/include/clang/Driver/Options.td
> >> +++ b/clang/include/clang/Driver/Options.td
> >> @@ -979,6 +979,9 @@ def fno_sanitize_EQ : CommaJoined<["-"], "fno-
> >> sanitize=">, Group,  def fsanitize_blacklist :
> >> Joined<["- "], "fsanitize-blacklist=">,
> >>   Group,
> >>   HelpText<"Path to blacklist file for
> >> sanitizers">;
> >> +def fsanitize_system_blacklist : Joined<["-"],
> >> +"fsanitize-system-blacklist=">,
> >> +  HelpText<"Path to system blacklist file for sanitizers">,
> >> +  Flags<[CC1Option]>;
> >> def fno_sanitize_blacklist : Flag<["-"], "fno-sanitize-blacklist">,
> >>  Group,
> >>  HelpText<"Don't use blacklist file for
> >> sanitizers">;
> >>
> >> diff  --git a/clang/include/clang/Driver/SanitizerArgs.h
> >> b/clang/include/clang/Driver/SanitizerArgs.h
> >> index c37499e0f201..0aebf8cb225d 100644
> >> --- a/clang/include/clang/Driver/SanitizerArgs.h
> >> +++ b/clang/include/clang/Driver/SanitizerArgs.h
> >> @@ -25,8 +25,8 @@ class SanitizerArgs {
> >>   SanitizerSet RecoverableSanitizers;
> >>   SanitizerSet TrapSanitizers;
> >>
> >> -  std::vector BlacklistFiles;
> >> -  std::vector ExtraDeps;
> >> +  std::vector UserBlacklistFiles;
> >> + std::vector SystemBlacklistFiles;
> >>   int CoverageFeatures = 0;
> >>   int MsanTrackOrigins = 0;
> >>   bool MsanUseAfterDtor = true;
> >>
> >> diff  --git a/clang/lib/Driver/SanitizerArgs.cpp
> >> b/clang/lib/Driver/SanitizerArgs.cpp
> >> index cc6c5e6ef438..8937197c253c 100644
> >> --- a/clang/lib/Driver/SanitizerArgs.cpp
> >> +++ b/clang/lib/Driver/SanitizerArgs.cpp
> >> @@ -557,29 +557,35 @@ SanitizerArgs::SanitizerArgs(const ToolChain
> >> &TC,
> >>
> >>   // Setup blacklist files.
> >>   // Add default blacklist from resource directory.
> >> -  addDefaultBlacklists(D, Kinds, BlacklistFiles);
> >> +  addDefaultBlacklists(D, Kinds, SystemBlacklistFiles);
> >>   // Parse -f(no-)sanitize-blacklist options.
> >>   for (const auto *Arg : Args) {
> >> if (Arg->getOption().matches(options::OPT_fsani

RE: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a dependency in cc1

2019-11-07 Thread Voss, Matthew via cfe-commits

Hi Jan,

It looks like this commit is causing DFSAN failures on the sanitizer bots and 
our internal CI. Could you take a look?

http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/24312/steps/64-bit%20check-dfsan/logs/stdio

Thanks,
Matthew

> -Original Message-
> From: cfe-commits  On Behalf Of Jan
> Korous via cfe-commits
> Sent: Thursday, November 7, 2019 2:07 PM
> To: cfe-commits@lists.llvm.org
> Subject: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a
> dependency in cc1
> 
> 
> Author: Jan Korous
> Date: 2019-11-07T14:06:43-08:00
> New Revision: 03b84e4f6d0e1c04f22d69cc445f36e1f713beb4
> 
> URL: https://github.com/llvm/llvm-
> project/commit/03b84e4f6d0e1c04f22d69cc445f36e1f713beb4
> DIFF: https://github.com/llvm/llvm-
> project/commit/03b84e4f6d0e1c04f22d69cc445f36e1f713beb4.diff
> 
> LOG: [clang] Report sanitizer blacklist as a dependency in cc1
> 
> Previously these were reported from the driver which blocked clang-scan-
> deps from getting the full set of dependencies from cc1 commands.
> 
> Also the default sanitizer blacklist that is added in driver was never
> reported as a dependency. I introduced -fsanitize-system-blacklist cc1
> option to keep track of which blacklists were user-specified and which
> were added by driver and clang -MD now also reports system blacklists as
> dependencies.
> 
> Differential Revision: https://reviews.llvm.org/D69290
> 
> Added:
> 
> 
> Modified:
> clang/include/clang/Driver/Options.td
> clang/include/clang/Driver/SanitizerArgs.h
> clang/lib/Driver/SanitizerArgs.cpp
> clang/lib/Frontend/CompilerInvocation.cpp
> clang/test/Driver/fsanitize-blacklist.c
> clang/test/Frontend/dependency-gen.c
> 
> Removed:
> 
> 
> 
> ##
> ##
> diff  --git a/clang/include/clang/Driver/Options.td
> b/clang/include/clang/Driver/Options.td
> index dcd2976a97f2..c2e30a16b8da 100644
> --- a/clang/include/clang/Driver/Options.td
> +++ b/clang/include/clang/Driver/Options.td
> @@ -979,6 +979,9 @@ def fno_sanitize_EQ : CommaJoined<["-"], "fno-
> sanitize=">, Group,  def fsanitize_blacklist : Joined<["-
> "], "fsanitize-blacklist=">,
>Group,
>HelpText<"Path to blacklist file for
> sanitizers">;
> +def fsanitize_system_blacklist : Joined<["-"],
> +"fsanitize-system-blacklist=">,
> +  HelpText<"Path to system blacklist file for sanitizers">,
> +  Flags<[CC1Option]>;
>  def fno_sanitize_blacklist : Flag<["-"], "fno-sanitize-blacklist">,
>   Group,
>   HelpText<"Don't use blacklist file for
> sanitizers">;
> 
> diff  --git a/clang/include/clang/Driver/SanitizerArgs.h
> b/clang/include/clang/Driver/SanitizerArgs.h
> index c37499e0f201..0aebf8cb225d 100644
> --- a/clang/include/clang/Driver/SanitizerArgs.h
> +++ b/clang/include/clang/Driver/SanitizerArgs.h
> @@ -25,8 +25,8 @@ class SanitizerArgs {
>SanitizerSet RecoverableSanitizers;
>SanitizerSet TrapSanitizers;
> 
> -  std::vector BlacklistFiles;
> -  std::vector ExtraDeps;
> +  std::vector UserBlacklistFiles;
> + std::vector SystemBlacklistFiles;
>int CoverageFeatures = 0;
>int MsanTrackOrigins = 0;
>bool MsanUseAfterDtor = true;
> 
> diff  --git a/clang/lib/Driver/SanitizerArgs.cpp
> b/clang/lib/Driver/SanitizerArgs.cpp
> index cc6c5e6ef438..8937197c253c 100644
> --- a/clang/lib/Driver/SanitizerArgs.cpp
> +++ b/clang/lib/Driver/SanitizerArgs.cpp
> @@ -557,29 +557,35 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
> 
>// Setup blacklist files.
>// Add default blacklist from resource directory.
> -  addDefaultBlacklists(D, Kinds, BlacklistFiles);
> +  addDefaultBlacklists(D, Kinds, SystemBlacklistFiles);
>// Parse -f(no-)sanitize-blacklist options.
>for (const auto *Arg : Args) {
>  if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) {
>Arg->claim();
>std::string BLPath = Arg->getValue();
>if (llvm::sys::fs::exists(BLPath)) {
> -BlacklistFiles.push_back(BLPath);
> -ExtraDeps.push_back(BLPath);
> +UserBlacklistFiles.push_back(BLPath);
>} else {
>  D.Diag(clang::diag::err_drv_no_such_file) << BLPath;
>}
>  } else if (Arg-
> >getOption().matches(options::OPT_fno_sanitize_blacklist)) {
>Arg->claim();
> -  BlacklistFiles.clear();
> -  ExtraDeps.clear();
> +  UserBlacklistFiles.clear();
> +  SystemBlacklistFiles.clear();
>  }
>}
>// Validate blacklists format.
>{
>  std::string BLError;
>  std::unique_ptr SCL(
> -llvm::SpecialCaseList::create(BlacklistFiles, BLError));
> +llvm::SpecialCaseList::create(UserBlacklistFiles, BLError));
> +if (!SCL.get())
> +  D.Diag(clang::diag::err_drv_malformed_sanitizer_blacklist) <<
> + BLError;  }  {
> +std::string BLError;
> +std::unique_ptr SCL(
> +llvm::Speci

RE: [clang] 1029747 - [Sema] Fix null pointer dereference handleAlwaysInlineAttr.

RE: [clang] 1029747 - [Sema] Fix null pointer dereference handleAlwaysInlineAttr.

RE: [clang] 589ce5f - [DebugInfo] Move constructor homing case in shouldOmitDefinition.

RE: [clang] 589ce5f - [DebugInfo] Move constructor homing case in shouldOmitDefinition.

Re: [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics

Re: [clang-tools-extra] 9b88a19 - [clangd] Add CSV export for trace metrics

RE: [clang-tools-extra] ebdb98f - [clang-tidy] Move fuchsia-restrict-system-includes to portability module for general use.

RE: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a dependency in cc1

RE: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a dependency in cc1

RE: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a dependency in cc1

RE: [clang] 03b84e4 - [clang] Report sanitizer blacklist as a dependency in cc1

11 matches

Site Navigation

Mail list logo

Footer information