public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Dongao Guo <dongao.guo@intel.com>, edk2-devel@lists.01.org
Cc: Liming Gao <liming.gao@intel.com>
Subject: Re: [PATCH v5] MdeModulePkg/RegularExpressionDxe:disable wraning to pass gcc4.8 build
Date: Wed, 10 Oct 2018 18:01:08 +0200	[thread overview]
Message-ID: <75d4d50c-9a5e-dbe0-6200-9e83a0efc395@redhat.com> (raw)
In-Reply-To: <1539157143-7548-1-git-send-email-dongao.guo@intel.com>

On 10/10/18 09:39, Dongao Guo wrote:
> There are three warnings reported by GCC 4.8 and the later GCC release
> are workaround with them.
> And all the three warnings are invalid,so I just disable warnings rather
> than fix them at now.
> 
> Following is the analysis from Laszlo Ersek.
> (1)
> 
>> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c: In
>> function 'compile_length_tree':
>> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regcomp.c:1516:7:
>> warning: 'len' may be used uninitialized in this function
>> [-Wmaybe-uninitialized]
>>    int len;
>>        ^
> 
> I think this is an invalid warning; the type of the controlling expression
> (node->type) is enum GimmickType, and the case labels cover all values of
> the enum. An assert(0) could be added, I guess, but again, upstream
> Oniguruma would be justified to reject the idea.
> 
> (2)
> 
>> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c: In
>> function 'parse_callout_args.isra.10.constprop.30':
>> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c:6753:25:
>> warning: 'rl' may be used uninitialized in this function
>> [-Wmaybe-uninitialized]
>>   vals[n].l = rl;
>>               ^
> 
> This warning is invalid, given:
> 
>   6749    if (cn > 0) {
>   6750      long rl;
>   6751      r = parse_long(enc, buf, bufend, 1, LONG_MAX, &rl);
>   6752      if (r == ONIG_NORMAL) {
>   6753        vals[n].l = rl;
> 
> Because parse_long() only returns ONIG_NORMAL after it sets (*rl).
> 
> (3)
> 
>> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c: In
>> function 'parse_callout_of_name.constprop.29':
>> MdeModulePkg/Universal/RegularExpressionDxe/Oniguruma/regparse.c:6861:38:
>> warning: 'tag_end' may be used uninitialized in this function
>> [-Wmaybe-uninitialized]
>>      if (! is_allowed_callout_tag_name(enc, tag_start, tag_end))
> 
> This warning is also invalid, given:
> 
>   6852    if (c == '[') {
>   6853      if (PEND) return ONIGERR_END_PATTERN_IN_GROUP;
>   6854      tag_start = p;
>   6855      while (! PEND) {
>   6856        if (PEND) return ONIGERR_END_PATTERN_IN_GROUP;
>   6857        tag_end = p;
>   6858        PFETCH_S(c);
>   6859        if (c == ']') break;
>   6860      }
>   6861      if (! is_allowed_callout_tag_name(enc, tag_start, tag_end))
>   6862        return ONIGERR_INVALID_CALLOUT_TAG_NAME;
>   6863
> 
> To see that, first we should note:
>  #define PEND         (p < end ?  0 : 1)
> 
> therefore PEND doesn't change if neither "p" nor "end" change.
> 
> Second, when we reach line 6855 (the "while") for the very first time,
> (!PEND) is certainly true (i.e., PEND is false), because otherwise we
> would have bailed at line 6853. Therefore we reach line 6857, and assign
> "tag_end". Regardless of whether we iterate zero or more *additional*
> times around the loop, "tag_end" will have been set, whenever we reach
> line 6861.
> 
> Cc: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dongao Guo <dongao.guo@intel.com>
> ---
>  MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
> index 16e91bd..07bc02e 100644
> --- a/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
> +++ b/MdeModulePkg/Universal/RegularExpressionDxe/RegularExpressionDxe.inf
> @@ -109,3 +109,6 @@
>  
>    # Oniguruma: error: variable 'fp' set but not used
>    GCC:*_*_*_CC_FLAGS = -Wno-error=unused-but-set-variable
> +
> +  # Oniguruma: tag_end in parse_callout_of_name
> +  GCC:*_*_*_CC_FLAGS = -Wno-error=maybe-uninitialized
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


      reply	other threads:[~2018-10-10 16:01 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-10  7:39 [PATCH v5] MdeModulePkg/RegularExpressionDxe:disable wraning to pass gcc4.8 build Dongao Guo
2018-10-10 16:01 ` Laszlo Ersek [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=75d4d50c-9a5e-dbe0-6200-9e83a0efc395@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox