[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-dev
Subject:    Re: [cfe-dev] [analyzer]
From:       Artem Dergachev via cfe-dev <cfe-dev () lists ! llvm ! org>
Date:       2018-10-25 20:32:04
Message-ID: 598a3036-67e6-eecf-ce6a-cb7a07c0dd81 () gmail ! com
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


Ah, my memory of a surgeonfish never fails me.
Yeah, i guess just take it over.

On 10/25/18 2:55 AM, Gábor Horváth wrote:
> Given the lack of recent activity feel free to commandeer the 
> revision. I think it should be quite close to be merged.
>
> On Thu, 25 Oct 2018 at 10:30, Alexander Zaitsev <zamazan4ik@tut.by 
> <mailto:zamazan4ik@tut.by>> wrote:
>
>     Didn't know that this check is already implemented. I think I can
>     continue this work (seems like original author of the change don't
>     work on it now). What do you think?
>
>     25.10.2018 6:34, Gábor Horváth пишет:
>>     Hi!
>>
>>     Do you have something in mind like this:
>>     https://reviews.llvm.org/D33672 ?
>>
>>     Regards,
>>     Gábor
>>
>>     Artem Dergachev via cfe-dev <cfe-dev@lists.llvm.org
>>     <mailto:cfe-dev@lists.llvm.org>> ezt írta (időpont: 2018. okt.
>>     25., Cs 4:32):
>>
>>         Hi,
>>
>>         Your overall plan sounds good, and i believe that such
>>         checker will be
>>         very useful, i'd love to have such check in the analyzer. If
>>         you want to
>>         post it upstream, i encourage you to start early by publishing
>>         prototypes on Phabricator for code reviews, even when you
>>         think they're
>>         not ready, just because code reviews are cool!
>>
>>         Path-sensitive analysis is indeed useful here because
>>         sometimes it's not
>>         immediately obvious from the code which values are possible
>>         for the
>>         sub-expression. Defining the buggy state can be a bit
>>         annoying because
>>         enum values can be non-contiguous and/or numerous; the former
>>         means that
>>         you'll potentially need to make a lot of State->assume(...)
>>         calls and
>>         see if none of the states with assumptions are null; the
>>         latter means
>>         that you'll need to make sure you identify segments of values
>>         to avoid
>>         calling assume() for *every* enum item. I also recommend
>>         ConstraintManager::assumeInclusiveRange() for direct
>>         assumptions over
>>         segments.
>>
>>         Your questions so far are AST questions, not specific to the
>>         analyzer.
>>         First of all, notice that every expression has a (qualified)
>>         type, which
>>         is the type of the value it evaluates to, and it can always
>>         be obtained
>>         via Expr::getType(). It may be void (eg., call expression for
>>         a function
>>         that returns void), but it's always there.
>>
>>         For cast-expression, as you might have already guessed, the
>>         type of the
>>         expression is the target type of the cast. Because, well,
>>         that's the
>>         whole point of the cast. This takes care of question 2.
>>
>>         Most functions return not raw Types but QualType objects that
>>         are types
>>         with qualifiers. You can always use the overloaded
>>         operator->() on the
>>         QualType to access the underlying Type; there's also
>>         QualType::getTypePtr(), but if you think you need it - most
>>         likely you
>>         don't.
>>
>>         Now, types, like statements or declarations, are a hierarchy.
>>         Some types
>>         are integer types, some are array or structure types, some
>>         are enum
>>         types. Enum types are represented by the EnumType class, to
>>         which you
>>         can try to dyn_cast<>() your type. Or, even better, use
>>         Type::getAs<>(),
>>         which can be accessed directly with operator->() on QualType.
>>
>>         If dyn_cast<>()/getAs<>() is successful - your type is an
>>         enum and you
>>         have a pointer to an EnumType object, so you can call
>>         EnumType::getDecl() to find the *declaration* of the enum in
>>         the code.
>>
>>         Also if the enum hides under a typedef, then the type
>>         wouldn't be an
>>         EnumType but it'd be a TypedefType, so the cast would fail.
>>         The easy way
>>         to get rid of typedefs is to do QualType::getCanonicalType().
>>
>>         Some declarations are forward declarations. You might need to do
>>         EnumDecl::getDefinition() to find the actual definition.
>>         Maybe you don't
>>         need that: i don't remember what operations are allowed on
>>         incomplete
>>         enum types.
>>
>>         Once you have your EnumDecl that is the definition, you can
>>         iterate over
>>         EnumDecl::enumerators() to see what values are present.
>>
>>         In Clang there are a lot more cast kinds of expressions than you
>>         probably expect, so you might want to take a look at the list
>>         of casts
>>         in clang/AST/OperationKinds.def and see which ones do you
>>         need; i don't
>>         think it'll be important at first, but just in case.
>>
>>         In order to quickly catch up on the basics, i also recommend
>>         the AST
>>         tutorial by Manuel Klimek at
>>         https://www.youtube.com/watch?v=VqCkCDFLSsc
>>
>>
>>         On 10/24/18 5:16 PM, Alexander Zaitsev via cfe-dev wrote:
>>         >
>>         > Hello. I am newbie in Clang Static Analyzer and I am trying
>>         to write
>>         > new Clang Static Analyzer check, which is aimed to find
>>         issues with
>>         > casting values to enum: if we cast anything which is no
>>         presented in
>>         > target enum it will be unspecified/undefined
>>         behavior(depends on C++
>>         > version).
>>         >
>>         > So my plan is:
>>         >
>>         >  1. Find all casts in source code. Seems like
>>         >     'check::PreStmt<CastExpr>>' it's what I need.
>>         >  2. In my implementation of `checkPreStmt` method I must
>>         get target
>>         >     type from CastExpr, but I don't know, how to do it -
>>         can you help
>>         >     with it?
>>         >  3. Then if target type in Cast is Enum, I must get all
>>         values from
>>         >     this Enum and compare it with all possible values which
>>         can be
>>         >     presented by CastExpr->getSubExpr() - here I don't know
>>         how to
>>         >     evaluate CastExpr->getSubExpr() and how to get all
>>         values from Enum.
>>         >
>>         > Do you have any ideas?
>>         >
>>         > --
>>         > Best regards,
>>         > Alexander Zaitsev
>>         >
>>         >
>>         > _______________________________________________
>>         > cfe-dev mailing list
>>         > cfe-dev@lists.llvm.org <mailto:cfe-dev@lists.llvm.org>
>>         > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>>         _______________________________________________
>>         cfe-dev mailing list
>>         cfe-dev@lists.llvm.org <mailto:cfe-dev@lists.llvm.org>
>>         http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>
>     -- 
>     Best regards,
>     Alexander Zaitsev
>


[Attachment #5 (text/html)]

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Ah, my memory of a surgeonfish never fails me.<br>
    Yeah, i guess just take it over.<br>
    <br>
    <div class="moz-cite-prefix">On 10/25/18 2:55 AM, Gábor Horváth
      wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CAPRL4a0dRk8e9xnKjbs1-HBmuvt5gAnhcmi6sjY3UWPRCkCEOA@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr">Given the lack of recent activity feel free to
        commandeer the revision. I think it should be quite close to be
        merged.<br>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr">On Thu, 25 Oct 2018 at 10:30, Alexander Zaitsev
          &lt;<a href="mailto:zamazan4ik@tut.by" moz-do-not-send="true">zamazan4ik@tut.by</a>&gt;
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0 0 0
          .8ex;border-left:1px #ccc solid;padding-left:1ex">
          <div text="#000000" bgcolor="#FFFFFF">
            <p>Didn't know that this check is already implemented. I
              think I can continue this work (seems like original author
              of the change don't work on it now). What do you think?<br>
            </p>
            <div class="m_3497051423702866874moz-cite-prefix">25.10.2018
              6:34, Gábor Horváth пишет:<br>
            </div>
            <blockquote type="cite">
              <div dir="auto">Hi!
                <div dir="auto"><br>
                </div>
                <div dir="auto">Do you have something in mind like
                  this: <a href="https://reviews.llvm.org/D33672"
                    target="_blank" moz-do-not-send="true">https://reviews.llvm.org/D33672</a>
                  ?</div>
                <div dir="auto"><br>
                </div>
                <div dir="auto">Regards,</div>
                <div dir="auto">Gábor</div>
              </div>
              <br>
              <div class="gmail_quote">
                <div dir="ltr">Artem Dergachev via cfe-dev &lt;<a
                    href="mailto:cfe-dev@lists.llvm.org" target="_blank"
                    moz-do-not-send="true">cfe-dev@lists.llvm.org</a>&gt;
                  ezt írta (időpont: 2018. okt. 25., Cs 4:32):<br>
                </div>
                <blockquote class="gmail_quote" style="margin:0 0 0
                  .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
                  <br>
                  Your overall plan sounds good, and i believe that such
                  checker will be <br>
                  very useful, i'd love to have such check in the
                  analyzer. If you want to <br>
                  post it upstream, i encourage you to start early by
                  publishing <br>
                  prototypes on Phabricator for code reviews, even when
                  you think they're <br>
                  not ready, just because code reviews are cool!<br>
                  <br>
                  Path-sensitive analysis is indeed useful here because
                  sometimes it's not <br>
                  immediately obvious from the code which values are
                  possible for the <br>
                  sub-expression. Defining the buggy state can be a bit
                  annoying because <br>
                  enum values can be non-contiguous and/or numerous; the
                  former means that <br>
                  you'll potentially need to make a lot of
                  State-&gt;assume(...) calls and <br>
                  see if none of the states with assumptions are null;
                  the latter means <br>
                  that you'll need to make sure you identify segments of
                  values to avoid <br>
                  calling assume() for *every* enum item. I also
                  recommend <br>
                  ConstraintManager::assumeInclusiveRange() for direct
                  assumptions over <br>
                  segments.<br>
                  <br>
                  Your questions so far are AST questions, not specific
                  to the analyzer. <br>
                  First of all, notice that every expression has a
                  (qualified) type, which <br>
                  is the type of the value it evaluates to, and it can
                  always be obtained <br>
                  via Expr::getType(). It may be void (eg., call
                  expression for a function <br>
                  that returns void), but it's always there.<br>
                  <br>
                  For cast-expression, as you might have already
                  guessed, the type of the <br>
                  expression is the target type of the cast. Because,
                  well, that's the <br>
                  whole point of the cast. This takes care of question
                  2.<br>
                  <br>
                  Most functions return not raw Types but QualType
                  objects that are types <br>
                  with qualifiers. You can always use the overloaded
                  operator-&gt;() on the <br>
                  QualType to access the underlying Type; there's also <br>
                  QualType::getTypePtr(), but if you think you need it -
                  most likely you <br>
                  don't.<br>
                  <br>
                  Now, types, like statements or declarations, are a
                  hierarchy. Some types <br>
                  are integer types, some are array or structure types,
                  some are enum <br>
                  types. Enum types are represented by the EnumType
                  class, to which you <br>
                  can try to dyn_cast&lt;&gt;() your type. Or, even
                  better, use Type::getAs&lt;&gt;(), <br>
                  which can be accessed directly with operator-&gt;() on
                  QualType.<br>
                  <br>
                  If dyn_cast&lt;&gt;()/getAs&lt;&gt;() is successful -
                  your type is an enum and you <br>
                  have a pointer to an EnumType object, so you can call
                  <br>
                  EnumType::getDecl() to find the *declaration* of the
                  enum in the code.<br>
                  <br>
                  Also if the enum hides under a typedef, then the type
                  wouldn't be an <br>
                  EnumType but it'd be a TypedefType, so the cast would
                  fail. The easy way <br>
                  to get rid of typedefs is to do
                  QualType::getCanonicalType().<br>
                  <br>
                  Some declarations are forward declarations. You might
                  need to do <br>
                  EnumDecl::getDefinition() to find the actual
                  definition. Maybe you don't <br>
                  need that: i don't remember what operations are
                  allowed on incomplete <br>
                  enum types.<br>
                  <br>
                  Once you have your EnumDecl that is the definition,
                  you can iterate over <br>
                  EnumDecl::enumerators() to see what values are
                  present.<br>
                  <br>
                  In Clang there are a lot more cast kinds of
                  expressions than you <br>
                  probably expect, so you might want to take a look at
                  the list of casts <br>
                  in clang/AST/OperationKinds.def and see which ones do
                  you need; i don't <br>
                  think it'll be important at first, but just in case.<br>
                  <br>
                  In order to quickly catch up on the basics, i also
                  recommend the AST <br>
                  tutorial by Manuel Klimek at <a
                    href="https://www.youtube.com/watch?v=VqCkCDFLSsc"
                    rel="noreferrer noreferrer" target="_blank"
                    moz-do-not-send="true">https://www.youtube.com/watch?v=VqCkCDFLSsc</a><br>
                  <br>
                  <br>
                  On 10/24/18 5:16 PM, Alexander Zaitsev via cfe-dev
                  wrote:<br>
                  &gt;<br>
                  &gt; Hello. I am newbie in Clang Static Analyzer and I
                  am trying to write <br>
                  &gt; new Clang Static Analyzer check, which is aimed
                  to find issues with <br>
                  &gt; casting values to enum: if we cast anything which
                  is no presented in <br>
                  &gt; target enum it will be unspecified/undefined
                  behavior(depends on C++ <br>
                  &gt; version).<br>
                  &gt;<br>
                  &gt; So my plan is:<br>
                  &gt;<br>
                  &gt;  1. Find all casts in source code. Seems like<br>
                  &gt;     'check::PreStmt&lt;CastExpr&gt;&gt;' it's
                  what I need.<br>
                  &gt;  2. In my implementation of `checkPreStmt` method
                  I must get target<br>
                  &gt;     type from CastExpr, but I don't know, how to
                  do it - can you help<br>
                  &gt;     with it?<br>
                  &gt;  3. Then if target type in Cast is Enum, I must
                  get all values from<br>
                  &gt;     this Enum and compare it with all possible
                  values which can be<br>
                  &gt;     presented by CastExpr-&gt;getSubExpr() - here
                  I don't know how to<br>
                  &gt;     evaluate CastExpr-&gt;getSubExpr() and how to
                  get all values from Enum.<br>
                  &gt;<br>
                  &gt; Do you have any ideas?<br>
                  &gt;<br>
                  &gt; -- <br>
                  &gt; Best regards,<br>
                  &gt; Alexander Zaitsev<br>
                  &gt;<br>
                  &gt;<br>
                  &gt; _______________________________________________<br>
                  &gt; cfe-dev mailing list<br>
                  &gt; <a href="mailto:cfe-dev@lists.llvm.org"
                    rel="noreferrer" target="_blank"
                    moz-do-not-send="true">cfe-dev@lists.llvm.org</a><br>
                  &gt; <a
                    href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev"
                    rel="noreferrer noreferrer" target="_blank"
                    moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
                  <br>
                  _______________________________________________<br>
                  cfe-dev mailing list<br>
                  <a href="mailto:cfe-dev@lists.llvm.org"
                    rel="noreferrer" target="_blank"
                    moz-do-not-send="true">cfe-dev@lists.llvm.org</a><br>
                  <a
                    href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev"
                    rel="noreferrer noreferrer" target="_blank"
                    moz-do-not-send="true">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
                </blockquote>
              </div>
            </blockquote>
            <pre class="m_3497051423702866874moz-signature" cols="72">-- 
Best regards,
Alexander Zaitsev</pre>
          </div>
        </blockquote>
      </div>
    </blockquote>
    <br>
  </body>
</html>

[Attachment #6 (text/plain)]

_______________________________________________
cfe-dev mailing list
cfe-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic