Quantcast

[9] RFR: 8175345: possible null pointer dereference defects

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[9] RFR: 8175345: possible null pointer dereference defects

Rahul Raghavan
Hi,

Please review following patch proposal to avoid
possible null pointer dereference warnings / errors for internal tests.

<webrev.01> - http://cr.openjdk.java.net/~rraghavan/8175345/webrev.01/


<jbs> - internal bug - https://bugs.openjdk.java.net/browse/JDK-8175345

-- Possible null-pointer-deref type issues reported with Internal tests
due to the case that "Function MultiNode::proj_out may return constant 'NULL'"

Already following type checks are in place -
.e.g.: [hotspot/src/share/vm/opto/multnode.cp]
      bool ProjNode::is_uncommon_trap_if_pattern(Deoptimization::DeoptReason reason) {

          ProjNode* other_proj = iff->proj_out(1-_con);
          if (other_proj == NULL) // Should never happen..
            return false;

So required explicit checks added at following reported locations in <webrev.01>,
      - IfNode::dominated_by - hotspot/src/share/vm/opto/ifnode.cpp
      - StringConcat::validate_control_flow - hotspot/src/share/vm/opto/stringopts.cpp
      - PhaseIdealLoop::intrinsify_fill - hotspot/src/share/vm/opto/loopTransform.cpp
      - CallNode::may_modify - hotspot/src/share/vm/opto/callnode.cpp

-- Found all valid reported defects issues with internal tests are indirectly related to above.
-- Confirmed no issues with testing -  jprt run (-testset hotspot).


Thanks,
Rahul
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR: 8175345: possible null pointer dereference defects

Tobias Hartmann-2
Hi Rahul,

On 08.03.2017 07:53, Rahul Raghavan wrote:
> <webrev.01> - http://cr.openjdk.java.net/~rraghavan/8175345/webrev.01/

Looks good to me!

> <jbs> - internal bug - https://bugs.openjdk.java.net/browse/JDK-8175345

Please open up the bug.

Thanks,
Tobias
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: [9] RFR: 8175345: possible null pointer dereference defects

Rahul Raghavan
Hi,

Thank you for the review Tobias.

> Please open up the bug.
Done.

Thanks,
Rahul

> -----Original Message-----
> From: Tobias Hartmann
> Sent: Wednesday, March 08, 2017 12:31 PM
> To: Rahul Raghavan; [hidden email]; Vladimir Kozlov; Zoltan Majo
> Subject: Re: [9] RFR: 8175345: possible null pointer dereference defects
>
> Hi Rahul,
>
> On 08.03.2017 07:53, Rahul Raghavan wrote:
> > <webrev.01> - http://cr.openjdk.java.net/~rraghavan/8175345/webrev.01/
>
> Looks good to me!
>
> > <jbs> - internal bug - https://bugs.openjdk.java.net/browse/JDK-8175345
>
> Please open up the bug.
>
> Thanks,
> Tobias
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR: 8175345: possible null pointer dereference defects

Vladimir Kozlov
In reply to this post by Rahul Raghavan
callnode.cpp  - proj_out() is expensive. I would suggest to check returns_pointer() first:

        Node* proj = returns_pointer() ? proj_out(TypeFunc::Parms) : NULL;
        if (proj != NULL) {
          const TypeInstPtr* inst_t = phase->type(proj)->isa_instptr();

Other files changes are fine.

thanks,
Vladimir

On 3/7/17 10:53 PM, Rahul Raghavan wrote:

> Hi,
>
> Please review following patch proposal to avoid
> possible null pointer dereference warnings / errors for internal tests.
>
> <webrev.01> - http://cr.openjdk.java.net/~rraghavan/8175345/webrev.01/
>
>
> <jbs> - internal bug - https://bugs.openjdk.java.net/browse/JDK-8175345
>
> -- Possible null-pointer-deref type issues reported with Internal tests
> due to the case that "Function MultiNode::proj_out may return constant 'NULL'"
>
> Already following type checks are in place -
> .e.g.: [hotspot/src/share/vm/opto/multnode.cp]
>       bool ProjNode::is_uncommon_trap_if_pattern(Deoptimization::DeoptReason reason) {
>
>           ProjNode* other_proj = iff->proj_out(1-_con);
>           if (other_proj == NULL) // Should never happen..
>             return false;
>
> So required explicit checks added at following reported locations in <webrev.01>,
>       - IfNode::dominated_by - hotspot/src/share/vm/opto/ifnode.cpp
>       - StringConcat::validate_control_flow - hotspot/src/share/vm/opto/stringopts.cpp
>       - PhaseIdealLoop::intrinsify_fill - hotspot/src/share/vm/opto/loopTransform.cpp
>       - CallNode::may_modify - hotspot/src/share/vm/opto/callnode.cpp
>
> -- Found all valid reported defects issues with internal tests are indirectly related to above.
> -- Confirmed no issues with testing -  jprt run (-testset hotspot).
>
>
> Thanks,
> Rahul
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RE: [9] RFR: 8175345: possible null pointer dereference defects

Rahul Raghavan
Hi,

Thank you Vladimir for review comments.
Yes agree with the proposed change in callnode.cpp.

Please find the revised <webrev.02> -
  http://cr.openjdk.java.net/~rraghavan/8175345/webrev.02/

Thanks,
Rahul

> -----Original Message-----
> From: Vladimir Kozlov
> Sent: Thursday, March 09, 2017 6:54 AM
> To: Rahul Raghavan; [hidden email]
> Subject: Re: [9] RFR: 8175345: possible null pointer dereference defects
>
> callnode.cpp  - proj_out() is expensive. I would suggest to check returns_pointer() first:
>
>         Node* proj = returns_pointer() ? proj_out(TypeFunc::Parms) : NULL;
>         if (proj != NULL) {
>           const TypeInstPtr* inst_t = phase->type(proj)->isa_instptr();
>
> Other files changes are fine.
>
> thanks,
> Vladimir
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [9] RFR: 8175345: possible null pointer dereference defects

Vladimir Kozlov
Good.

Thanks,
Vladimir

On 3/8/17 10:58 PM, Rahul Raghavan wrote:

> Hi,
>
> Thank you Vladimir for review comments.
> Yes agree with the proposed change in callnode.cpp.
>
> Please find the revised <webrev.02> -
>   http://cr.openjdk.java.net/~rraghavan/8175345/webrev.02/
>
> Thanks,
> Rahul
>
>> -----Original Message-----
>> From: Vladimir Kozlov
>> Sent: Thursday, March 09, 2017 6:54 AM
>> To: Rahul Raghavan; [hidden email]
>> Subject: Re: [9] RFR: 8175345: possible null pointer dereference defects
>>
>> callnode.cpp  - proj_out() is expensive. I would suggest to check returns_pointer() first:
>>
>>         Node* proj = returns_pointer() ? proj_out(TypeFunc::Parms) : NULL;
>>         if (proj != NULL) {
>>           const TypeInstPtr* inst_t = phase->type(proj)->isa_instptr();
>>
>> Other files changes are fine.
>>
>> thanks,
>> Vladimir
>>
Loading...