From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 083523858419 for ; Tue, 9 Aug 2022 20:02:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 083523858419 Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-443-E5FDgadPPGSc0fs9yy990A-1; Tue, 09 Aug 2022 16:02:55 -0400 X-MC-Unique: E5FDgadPPGSc0fs9yy990A-1 Received: by mail-qt1-f197.google.com with SMTP id i19-20020ac85e53000000b00342f05b902cso6586030qtx.7 for ; Tue, 09 Aug 2022 13:02:55 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:user-agent:references :in-reply-to:date:cc:to:from:subject:message-id:x-gm-message-state :from:to:cc; bh=N7jTRdw/EIZcfYwYE8vqJ0VEjSHiUauJSkr7L0qyf74=; b=fr5Muvi7rfWiGi8eEeXlLH2P3+k8AlulvwSwo5oFKFfT4M2F6ypE8c5gNwpUsdWWbV 1cr6Sha64BHGhy8Qs9zQWoDkatgzSLshGJNpJPfuNi2Bv/G+1ar7gP+v0dei2++tzQuq 9s4yvZdK/ZwcLYi4+rYJ3yjgOcj6J5nLR1FVlVGKNSU4Fe9wYTH1b378tHIiBC1dwt5L Emk3K5S58OITPMJR9vvljU8CRIFzT/Hqs/5eQ8i7nlbU8st1xnOReSbEgMh3vif2kWI/ Eo47dNFPDmIH1+PPauHPzdk0hPu2gfaaRfQEbay3fM2u8ZUDftQ9bDJFgg09VJ4NKvLW 9swQ== X-Gm-Message-State: ACgBeo0amYW8wKIF1YEYPSr1b0tjZ9lpe4ifg1BFdtuZifXbfBFJJP3B TJC1j8REo0BG62vVx1JnWAwMnrrfNnM+keiOrYgjalebDYBNQviTb8MJ2syEqVtg7SSAmo47E75 m61L2Sau2rcQ5Kx2qDw== X-Received: by 2002:ae9:e64b:0:b0:6b9:8566:ea5b with SMTP id x11-20020ae9e64b000000b006b98566ea5bmr2437186qkl.311.1660075375148; Tue, 09 Aug 2022 13:02:55 -0700 (PDT) X-Google-Smtp-Source: AA6agR7B6oV/pG/L8IFGZfyjDN1SERJ4FEKq884eCPiquukILMC0gjUPKS9oOqhiVAI5Cx06gEo7ZA== X-Received: by 2002:ae9:e64b:0:b0:6b9:8566:ea5b with SMTP id x11-20020ae9e64b000000b006b98566ea5bmr2437167qkl.311.1660075374893; Tue, 09 Aug 2022 13:02:54 -0700 (PDT) Received: from t14s.localdomain (c-73-69-212-193.hsd1.nh.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id x8-20020a05620a258800b006b5e45ff82csm12431568qko.93.2022.08.09.13.02.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Aug 2022 13:02:53 -0700 (PDT) Message-ID: Subject: Re: [PATCH] analyzer: fix ICE casued by dup2 in sm-fd.cc[PR106551] From: David Malcolm To: mirimnan017@gmail.com, gcc-patches@gcc.gnu.org Cc: Immad Mir Date: Tue, 09 Aug 2022 16:02:52 -0400 In-Reply-To: References: User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Aug 2022 20:02:58 -0000 On Tue, 2022-08-09 at 21:42 +0530, Immad Mir wrote: > This patch fixes the ICE caused by valid_to_unchecked_state, > at analyzer/sm-fd.cc by handling the m_start state in > check_for_dup. > > Tested lightly on x86_64. > > gcc/analyzer/ChangeLog: >         PR analyzer/106551 >         * sm-fd.cc (check_for_dup): handle the m_start >         state when transitioning the state of LHS >         of dup, dup2 and dup3 call. > > gcc/testsuite/ChangeLog: >         * gcc.dg/analyzer/fd-dup-1.c: New testcases. > > Signed-off-by: Immad Mir > --- >  gcc/analyzer/sm-fd.cc                    |  4 ++-- >  gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c | 28 > +++++++++++++++++++++++- >  2 files changed, 29 insertions(+), 3 deletions(-) > > diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc > index 8bb76d72b05..c8b9930a7b6 100644 > --- a/gcc/analyzer/sm-fd.cc > +++ b/gcc/analyzer/sm-fd.cc > @@ -983,7 +983,7 @@ fd_state_machine::check_for_dup (sm_context > *sm_ctxt, const supernode *node, >      case DUP_1: >        if (lhs) >         { > -         if (is_constant_fd_p (state_arg_1)) > +         if (is_constant_fd_p (state_arg_1) || state_arg_1 == > m_start) >             sm_ctxt->set_next_state (stmt, lhs, > m_unchecked_read_write); >           else >             sm_ctxt->set_next_state (stmt, lhs, > @@ -1011,7 +1011,7 @@ fd_state_machine::check_for_dup (sm_context > *sm_ctxt, const supernode *node, >        file descriptor i.e the first argument.  */ >        if (lhs) >         { > -         if (is_constant_fd_p (state_arg_1)) > +         if (is_constant_fd_p (state_arg_1) || state_arg_1 == > m_start) >             sm_ctxt->set_next_state (stmt, lhs, > m_unchecked_read_write); >           else >             sm_ctxt->set_next_state (stmt, lhs, > diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c > b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c > index eba2570568f..ed4d6de57db 100644 > --- a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c > +++ b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c > @@ -220,4 +220,30 @@ test_19 (const char *path, void *buf) >          close (fd); >      } >      > -} > \ No newline at end of file > +} > + > +void > +test_20 () > +{ > +    int m; > +    int fd = dup (m); /* { dg-warning "'dup' on possibly invalid > file descriptor 'm'" } */ > +    close (fd); > +} > + > +void > +test_21 () > +{ > +    int m; > +    int fd = dup2 (m, 1); /* { dg-warning "'dup2' on possibly > invalid file descriptor 'm'" } */ > +    close (fd); > +} > + > +void > +test_22 (int flags) > +{ > +    int m; > +    int fd = dup3 (m, 1, flags); /* { dg-warning "'dup3' on possibly > invalid file descriptor 'm'" } */ > +    close (fd); > +} Thanks for the updated patch. The test cases looked suspicious to me - I was wondering why the analyzer doesn't complain about the uninitialized values being passed to the various dup functions as parameters. So your test cases seem to have uncovered a hidden pre-existing bug in the analyzer's uninitialized value detection, which I've filed for myself to deal with as PR analyzer/106573. If you convert the "int m;" locals into an extern global, like in comment #0 of bug 106551, does that still trigger the crash on the unpatched sm-fd.cc? If so, then that's greatly preferable as a regression test, since otherwise I'll have to modify that test case when I fix bug 106573. Dave