Skip to content

Commit bff3c95

Browse files
paulbissfacebook-github-bot
authored andcommitted
Fix non-unique ProfPrologueIDs
Summary: In low pointer builds we use the 32 bit Func* as an ID instead of computing separate unique ID numbers for each function. This works fine in repo mode but in non-repo mode it means that FuncId is no longer unique (e.g. if I free a Func and allocate a new one it may be allocated at the same address and therefore share the same FuncId). This is mostly a theoretical concern as we don't run non-repo low pointer builds anywhere but we do have some tests that depend on being able to calculate unique ProfPrologueIDs in all build configurations. To fix that without expanding the Func structure store a side table of IDs in non-repo mode. Reviewed By: ricklavoie Differential Revision: D73924655 fbshipit-source-id: 3495f3ab3c8b941f5b95913e2e7dad21923673e3
1 parent 069c0d0 commit bff3c95

File tree

5 files changed

+34
-2
lines changed

5 files changed

+34
-2
lines changed

hphp/runtime/vm/func-id.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,17 @@
1616
#include <limits>
1717

1818
#include "hphp/runtime/vm/func-id.h"
19+
#include "hphp/runtime/vm/func.h"
1920

2021
namespace HPHP {
2122
///////////////////////////////////////////////////////////////////////////////
2223

2324
FuncId FuncId::Invalid = FuncId::fromInt(std::numeric_limits<FuncId::Int>::max());
2425
FuncId FuncId::Dummy = FuncId::fromInt(std::numeric_limits<FuncId::Int>::max() - 1);
2526

27+
#ifdef USE_LOWPTR
28+
FuncId::Int FuncId::toStableInt() const { return getFunc()->getStableId(); }
29+
#endif
30+
2631
///////////////////////////////////////////////////////////////////////////////
2732
}

hphp/runtime/vm/func-id.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,14 @@ struct FuncId {
5151
static FuncId fromInt(Int num) {
5252
return FuncId{Id(reinterpret_cast<const Func*>(num))};
5353
}
54+
Int toStableInt() const;
5455
#else
5556
using Id = uint32_t;
5657
Int toInt() const { return m_id; }
5758
constexpr static FuncId fromInt(Int num) {
5859
return FuncId{num};
5960
}
61+
Int toStableInt() const { return toInt(); }
6062
#endif
6163

6264
bool isInvalid() const { return m_id == Invalid.m_id; }

hphp/runtime/vm/func.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ static InitFiniNode s_funcVecReinit([]{
8686
UnsafeReinitEmptyAtomicLowPtrVector(
8787
Func::s_funcVec, Cfg::Eval::FuncCountHint);
8888
}, InitFiniNode::When::PostRuntimeOptions, "s_funcVec reinit");
89+
#else
90+
folly::ConcurrentHashMap<uint32_t, uint32_t> s_stableFuncIDs;
8991
#endif
9092

9193
static ServiceData::ExportedCounter* s_funcid_counter =
@@ -389,8 +391,21 @@ FuncId::Int Func::maxFuncIdNum() {
389391

390392
#ifdef USE_LOWPTR
391393
void Func::setNewFuncId() {
392-
s_nextFuncId.fetch_add(1, std::memory_order_acq_rel);
394+
auto const id = s_nextFuncId.fetch_add(1, std::memory_order_acq_rel);
393395
s_funcid_counter->increment();
396+
397+
if (!Cfg::Repo::Authoritative) {
398+
s_stableFuncIDs.insert_or_assign(
399+
static_cast<uint32_t>(reinterpret_cast<intptr_t>(this)), id);
400+
}
401+
}
402+
403+
uint32_t Func::getStableId() const {
404+
if (Cfg::Repo::Authoritative) return getFuncId().toInt();
405+
auto const it = s_stableFuncIDs.find(
406+
static_cast<uint32_t>(reinterpret_cast<intptr_t>(this)));
407+
assertx(it != s_stableFuncIDs.end());
408+
return it->second;
394409
}
395410

396411
const Func* Func::fromFuncId(FuncId id) {
@@ -413,6 +428,10 @@ void Func::setNewFuncId() {
413428
s_funcVec.set(m_funcId.toInt(), this);
414429
}
415430

431+
uint32_t Func::getStableId() const {
432+
return getFuncId().toInt();
433+
}
434+
416435
const Func* Func::fromFuncId(FuncId id) {
417436
assertx(id.toInt() < s_nextFuncId);
418437
auto const func = s_funcVec.get(id.toInt());

hphp/runtime/vm/func.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,12 @@ struct Func final {
258258
*/
259259
FuncId getFuncId() const;
260260

261+
/*
262+
* Get a function ID that is guaranteed not to be reused during the lifetime
263+
* of the process.
264+
*/
265+
uint32_t getStableId() const;
266+
261267
#ifndef USE_LOWPTR
262268
static constexpr size_t funcIdOffset() {
263269
return offsetof(Func, m_funcId);

hphp/runtime/vm/jit/prof-data.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -594,7 +594,7 @@ struct ProfData {
594594

595595
/* implicit */ operator uint64_t() const {
596596
assertx(nArgs >= 0);
597-
return (uint64_t(func.toInt()) << 32) | nArgs;
597+
return (uint64_t(func.toStableInt()) << 32) | nArgs;
598598
}
599599
};
600600

0 commit comments

Comments
 (0)