[Einstein Toolkit] #1821: Carpet may call object methods with this == NULL

List overview All Threads
Download

newer

older

[Einstein Toolkit] #1986: this...

[Einstein Toolkit] #2047: support...

Einstein Toolkit

17 Oct 2015 17 Oct '15

1:09 p.m.

#1821: Carpet may call object methods with this == NULL ----------------------+----------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: new Priority: optional | Milestone: Component: Carpet | Version: development version Keywords: | ----------------------+----------------------------------------------------- see pull request https://bitbucket.org/eschnett/carpet/pull-requests/7 /carpet-avoid-using-null-pointer-for-member/diff

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Show replies by date

Einstein Toolkit

17 Oct 17 Oct

1:09 p.m.

#1821: Carpet may call object methods with this == NULL -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: review Priority: optional | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: -----------------------+---------------------------------------------------- Changes (by rhaas):

* status: new => review

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:1 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

2 Jul 2 Jul

9:48 a.m.

#1821: Carpet may call object methods with this == NULL -------------------------+-------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: closed Priority: optional | Milestone: Component: Carpet | Version: development version Resolution: worksforme | Keywords: -------------------------+-------------------------------------------------- Changes (by rhaas):

* status: review => closed * resolution: => worksforme

Comment:

This is likely not worthwhile to address (if indeed it is a bug) unless we encounter a system where this breaks (with a SEGFAULT so it won't be a silent failure). See https://bitbucket.org/eschnett/carpet/pull-requests/7 /carpet-avoid-using-null-pointer-for-member/diff#comment-10904860

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:2 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

2 Mar 2 Mar

4:51 p.m.

#1821: Carpet may call object methods with this == NULL -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: reopened Priority: optional | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: -----------------------+---------------------------------------------------- Changes (by rhaas):

* status: closed => reopened * resolution: worksforme =>

Comment:

Fails in CarpetPeriodic test testperiodicinterp. Related to #1986 which is however only a warning.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:3 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

4:52 p.m.

#1821: Carpet may call object methods with this == NULL -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: reopened Priority: optional | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------

Comment (by rhaas):

I re-opened #1821 which had the original proposed fix. I'll put the updated fix with only a single new_typed_data into the old pull request.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:4 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

3 Mar 3 Mar

2:05 p.m.

#1821: Carpet may call object methods with this == NULL -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: reopened Priority: optional | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------

Comment (by eschnett):

I now recall the correction I put into place. The fix is applied at the calling side (obviously). Instead of creating a temporary object, we're using another object that already exists at the caller, e.g. {{{src}}} instead of {{{dst}}}.

I thought this fix was already committed. Maybe I only applied it to a branch, or maybe I missed a calling site?

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:4 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

2:16 p.m.

#1821: Carpet may call object methods with this == NULL -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: reopened Priority: optional | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------

Comment (by rhaas):

maybe, here's another way about it: https://bitbucket.org/eschnett/carpet /pull-requests/7/carpet-avoid-using-null-pointer-for-member/diff

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:5 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

4:01 p.m.

#1821: Carpet may call object methods with this == NULL -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: reopened Priority: optional | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------

Comment (by eschnett):

(1) I don't think that CarpetLib itself calls this method, so the changes to {{{ggf.cc}}} should not be necessary. (2) In {{{PeriodicCarpet}}}, you could use {{{src}}} instead of {{{fake_data_pointer()}}}. (3) The initialization of the static variable in {{{fake_data_pointer}}} is not thread-safe. This has actually become an issue when using Qthreads for parallelization.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:6 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

15 Mar 15 Mar

3:37 p.m.

#1821: Carpet may call object methods with this == NULL -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: reopened Priority: optional | Milestone: Component: Carpet | Version: development version Resolution: | Keywords: -----------------------+----------------------------------------------------

Comment (by rhaas):

...

(1) I don't think that CarpetLib itself calls this method, so the

changes to {{{ggf.cc}}} should not be necessary.

I added a comment to the code diff: https://bitbucket.org/eschnett/carpet/pull-requests/7/carpet-avoid-using- null-pointer-for-member/diff#comment-33242870

...

(2) In {{{PeriodicCarpet}}}, you could use {{{src}}} instead of

{{{fake_data_pointer()}}}. Could be done. That seems odd though.

...

(3) The initialization of the static variable in {{{fake_data_pointer}}}

is not thread-safe. This has actually become an issue when using Qthreads for parallelization. Correct. At the time I reported this CarpetLib was not yet thread safe (ie none of the Timers was) so it did not matter. Thread safe would be either using new() or the src trick that you suggest or making transfer_from a static member function that takes the current dst as eg its first argument.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:7 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

11 May 11 May

8:26 a.m.

* status: reopened => review

Comment:

I updated the pull request with a version that introduces static functions for gdata::copy_from and gdata::transfer_from and renames them to copy_data and transfer_data. This requires changes in all code using those functions since the originals are gone (could be restored but the current state brings attention to possible code parts that may also pass in a NULL this pointer).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:8 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9 Jun 9 Jun

9:08 a.m.

Comment (by rhaas):

Erik: I think this should be reviewed before the next release as otherwise CarpetPeriodic fails to run when compiled with newer compilers (gcc 6.0 at least).

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:9 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

9:33 a.m.

#1821: Carpet may call object methods with this == NULL -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: review Priority: optional | Milestone: ET_2017_05 Component: Carpet | Version: development version Resolution: | Keywords: -----------------------+---------------------------------------------------- Changes (by rhaas):

* milestone: => ET_2017_05

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:10 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

12 Jun 12 Jun

9:55 a.m.

#1821: Carpet may call object methods with this == NULL -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: reviewed_ok Priority: optional | Milestone: ET_2017_05 Component: Carpet | Version: development version Resolution: | Keywords: -----------------------+---------------------------------------------------- Changes (by eschnett):

* status: review => reviewed_ok

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:11 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

Einstein Toolkit

13 Jun 13 Jun

1:51 p.m.

#1821: Carpet may call object methods with this == NULL -----------------------+---------------------------------------------------- Reporter: rhaas | Owner: eschnett Type: defect | Status: closed Priority: optional | Milestone: ET_2017_05 Component: Carpet | Version: development version Resolution: fixed | Keywords: -----------------------+---------------------------------------------------- Changes (by rhaas):

* status: reviewed_ok => closed * resolution: => fixed

Comment:

Thank you. Applied as git hash 474a09de0c1cd56c00492400fe5304c8e9591cbd "PeriodicCarpet: avoid using NULL this pointer for gdata::copy_from" of Carpet.

-- Ticket URL: https://trac.einsteintoolkit.org/ticket/1821#comment:12 Einstein Toolkit http://einsteintoolkit.org The Einstein Toolkit

3405

Age (days ago)

4010

Last active (days ago)

trac@lists.einsteintoolkit.org

13 comments

1 participants

tags (0)

participants (1)

Einstein Toolkit