Bug 88561 - [8/9 Regression] PGO devirtualization miscompilation of firefox
Summary: [8/9 Regression] PGO devirtualization miscompilation of firefox
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: ipa (show other bugs)
Version: 9.0
: P2 normal
Target Milestone: 8.3
Assignee: Not yet assigned to anyone
URL:
Keywords: wrong-code
Depends on:
Blocks: mozillametabug
  Show dependency treegraph
 
Reported: 2018-12-20 11:36 UTC by Jakub Jelinek
Modified: 2019-01-04 03:26 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2018-12-20 00:00:00


Attachments
Fix I am testing (2.25 KB, patch)
2018-12-20 14:36 UTC, Jan Hubicka
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jakub Jelinek 2018-12-20 11:36:32 UTC
g++ -O3 -fprofile-generate -o test{1,.C}; ./test1; g++ -O3 -fprofile-use -o test{2,.C}; ./test2
aborts when using g++ 8 or current trunk.

struct nsISupports
{
  virtual int QueryInterface (const int &aIID, void **aInstancePtr) = 0;
  virtual __attribute__((noinline, noclone)) unsigned AddRef (void) = 0;
  virtual unsigned Release (void) = 0;
};

struct nsIObserver : public nsISupports
{
  virtual int Observe (nsISupports * aSubject, const char *aTopic, const unsigned short *aData) = 0;
};

struct nsISupportsWeakReference : public nsISupports
{
  virtual int GetWeakReference (void **_retval) = 0;
};

struct nsSupportsWeakReference : public nsISupportsWeakReference
{
  nsSupportsWeakReference () : mProxy (0) {}
  virtual int GetWeakReference (void **_retval) override { return 0; }
  ~nsSupportsWeakReference () {}
  void NoticeProxyDestruction () { mProxy = nullptr; }
  void *mProxy;
  void ClearWeakReferences ();
  bool HasWeakReferences () const { return !!mProxy; }
};

struct mozIPersonalDictionary : public nsISupports
{
  virtual int Load (void) = 0;
  virtual int Save (void) = 0;
  virtual int GetWordList (void **aWordList) = 0;
  virtual int Check (const int &word, bool * _retval) = 0;
  virtual int AddWord (const int &word) = 0;
  virtual int RemoveWord (const int &word) = 0;
  virtual int IgnoreWord (const int &word) = 0;
  virtual int EndSession (void) = 0;
};

struct mozPersonalDictionary final
  : public mozIPersonalDictionary, public nsIObserver, public nsSupportsWeakReference
{
  virtual int QueryInterface (const int &aIID, void **aInstancePtr) override;
  virtual __attribute__((noinline, noclone)) unsigned AddRef (void) override;
  virtual unsigned Release (void) override;
  unsigned long mRefCnt;
  virtual int Load (void) override { return 0; }
  virtual int Save (void) override { return 0; }
  virtual int GetWordList (void **aWordList) override { return 0; }
  virtual int Check (const int &word, bool * _retval) override { return 0; }
  virtual int AddWord (const int &word) override { return 0; }
  virtual int RemoveWord (const int &word) override { return 0; }
  virtual int IgnoreWord (const int &word) override { return 0; }
  virtual int EndSession (void) override { return 0; }
  virtual int Observe (nsISupports * aSubject, const char *aTopic, const unsigned short *aData) override { return 0; }
  mozPersonalDictionary () : mRefCnt(0) {}
  int Init () { return 0; }
  virtual ~mozPersonalDictionary () {}
  bool mIsLoaded;
  bool mSavePending;
  void *mFile;
  char mMonitor[96];
  char mMonitorSave[96];
  char mDictionaryTable[32];
  char mIgnoreTable[32];
};

unsigned
mozPersonalDictionary::AddRef (void)
{
  unsigned count = ++mRefCnt;
  return count;
}

unsigned
mozPersonalDictionary::Release (void)
{
  unsigned count = --mRefCnt;
  if (count == 0)
    {
      mRefCnt = 1;
      delete (this);
      return 0;
    }
  return count;
}

int
mozPersonalDictionary::QueryInterface (const int &aIID, void **aInstancePtr)
{
  nsISupports *foundInterface;
  if (aIID == 122)
    foundInterface = static_cast <mozIPersonalDictionary *>(this);
  else
    foundInterface = static_cast <nsISupportsWeakReference *>(this);
  int status;
  foundInterface->AddRef ();
  *aInstancePtr = foundInterface;
  return status;
}

__attribute__((noipa)) int
foo (nsISupports *p, const int &i)
{
  void *q;
  return p->QueryInterface (i, &q);
}

int
main ()
{
  mozPersonalDictionary m;
  int j = 123;
  for (int i = 0; i < 100000; i++)
    foo (static_cast <nsISupportsWeakReference *>(&m), j);
  if (m.mRefCnt != 100000)
    __builtin_abort ();
}

Testcase reduced from firefox https://bugzilla.mozilla.org/show_bug.cgi?id=1495742
The problem is that in the _ZThn8_N21mozPersonalDictionary14QueryInterfaceERKiPPv thunk we devirtualize incorrectly, call _ZThn8_N21mozPersonalDictionary6AddRefEv when we should have called _ZThn16_N21mozPersonalDictionary6AddRefEv instead.
Comment 1 Jan Hubicka 2018-12-20 14:36:19 UTC
Created attachment 45273 [details]
Fix I am testing
Comment 2 Jan Hubicka 2018-12-21 19:13:38 UTC
Author: hubicka
Date: Fri Dec 21 19:13:06 2018
New Revision: 267338

URL: https://gcc.gnu.org/viewcvs?rev=267338&root=gcc&view=rev
Log:

	PR ipa/88561
	* ipa-polymorphic-call.c
	(ipa_polymorphic_call_context::ipa_polymorphic_call_context): Handle
	arguments of thunks correctly.
	(ipa_polymorphic_call_context::get_dynamic_context): Be ready for
	NULL instance pinter.
	* lto-cgraph.c (lto_output_node): Always stream thunk info.

Added:
    trunk/gcc/testsuite/g++.dg/tree-prof/devirt.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-polymorphic-call.c
    trunk/gcc/lto-cgraph.c
    trunk/gcc/testsuite/ChangeLog
Comment 3 Martin Liška 2018-12-27 12:33:31 UTC
Author: marxin
Date: Thu Dec 27 12:33:00 2018
New Revision: 267433

URL: https://gcc.gnu.org/viewcvs?rev=267433&root=gcc&view=rev
Log:
Backport r267338

2018-12-27  Martin Liska  <mliska@suse.cz>

	Backport from mainline
	2018-12-15  Jan Hubicka  <hubicka@ucw.cz>

	PR ipa/88561
	* ipa-polymorphic-call.c
	(ipa_polymorphic_call_context::ipa_polymorphic_call_context): Handle
	arguments of thunks correctly.
	(ipa_polymorphic_call_context::get_dynamic_context): Be ready for
	NULL instance pinter.
	* lto-cgraph.c (lto_output_node): Always stream thunk info.
2018-12-27  Martin Liska  <mliska@suse.cz>

	Backport from mainline
	2018-12-15  Jan Hubicka  <hubicka@ucw.cz>

	PR ipa/88561
	* g++.dg/tree-prof/devirt.C: New testcase.

Added:
    branches/gcc-8-branch/gcc/testsuite/g++.dg/tree-prof/devirt.C
Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/ipa-polymorphic-call.c
    branches/gcc-8-branch/gcc/lto-cgraph.c
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
Comment 4 Martin Liška 2018-12-27 12:33:46 UTC
Fixed on all active branches.
Comment 5 Jakub Jelinek 2019-01-02 09:26:31 UTC
Author: jakub
Date: Wed Jan  2 09:25:59 2019
New Revision: 267507

URL: https://gcc.gnu.org/viewcvs?rev=267507&root=gcc&view=rev
Log:
	PR ipa/88561
	* g++.dg/tree-prof/devirt.C: Expect _ZThn16 only for lp64 and llp64
	targets and expect _ZThn8 for ilp32 targets.

Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/tree-prof/devirt.C
Comment 6 Martin Liška 2019-01-03 08:49:36 UTC
Author: marxin
Date: Thu Jan  3 08:49:04 2019
New Revision: 267546

URL: https://gcc.gnu.org/viewcvs?rev=267546&root=gcc&view=rev
Log:
Backport r267507

2019-01-03  Martin Liska  <mliska@suse.cz>

	Backport from mainline
	2019-01-02  Jakub Jelinek  <jakub@redhat.com>

	PR ipa/88561
	* g++.dg/tree-prof/devirt.C: Expect _ZThn16 only for lp64 and llp64
	targets and expect _ZThn8 for ilp32 targets.

Modified:
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
    branches/gcc-8-branch/gcc/testsuite/g++.dg/tree-prof/devirt.C