Skip to content

Commit

Permalink
Protect error implication from ENA out-of-bounds.
Browse files Browse the repository at this point in the history
Not a perfect solution but catch unwinding panics from using Obligations
outside of their original inference context.
  • Loading branch information
gavinleroy committed Mar 1, 2024
1 parent 41a256b commit b59d23f
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 19 deletions.
28 changes: 21 additions & 7 deletions crates/argus/src/analysis/tls.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Thread local storage for storing data processed in rustc.
use std::cell::RefCell;
use std::{cell::RefCell, panic};

use index_vec::IndexVec;
use rustc_data_structures::fx::FxIndexMap;
Expand Down Expand Up @@ -45,25 +45,39 @@ pub fn store_obligation(obl: Provenance<Obligation>) {
})
}

// TODO: using `infcx` for error implication panics, but using
// stored contexts doesn't. Investigate why, as this certainly
// isn't a "solution."
pub fn drain_implied_ambiguities<'tcx>(
infcx: &InferCtxt<'tcx>,
_infcx: &InferCtxt<'tcx>,
obligation: &PredicateObligation<'tcx>,
) {
use std::panic::AssertUnwindSafe;
OBLIGATIONS.with(|obls| {
obls.borrow_mut().retain(|o| {
let mut obls = obls.borrow_mut();
obls.retain(|provenance| {
// Drain all elements that are:
// 1. Ambiguous, and--
// 2. Implied by the passed obligation
let should_remove = o.result.is_maybe()
&& o
let should_remove = provenance.result.is_maybe()
&& provenance
.full_data
.map(|idx| {
unsafe_tls::borrow_in(idx, |fdata| {
fdata.infcx.universe() == infcx.universe()
&& infcx.error_implies(
// NOTE: using the inference context in this was is problematic as
// we can't know for sure whether variables won't be leaked. (I.e.,
// used in a context they don't live in.) The open snapshot check is
// trying to mitigate this happening, but it's not foolproof and we
// certainly don't want to crash the program.
// Furthermore, this closure is unwind safe because the inference contexts
// are forked, and no one outside this thread can access them.
panic::catch_unwind(AssertUnwindSafe(|| {
fdata.infcx.error_implies(
obligation.predicate.clone(),
fdata.obligation.predicate.clone(),
)
}))
.unwrap_or(false)
})
})
.unwrap_or(false);
Expand Down
13 changes: 9 additions & 4 deletions ide/packages/panoptes/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import React, { useEffect, useState } from "react";

import Workspace from "./Workspace";
import { highlightedObligation } from "./signals";
import { bringToFront } from "./utilities/func";

const App = ({
initialData,
Expand Down Expand Up @@ -44,10 +45,14 @@ const App = ({

case "open-file": {
return setOpenFiles(currFiles => {
if (_.find(currFiles, ([filename, _]) => filename === payload.file)) {
return currFiles;
const idx = _.findIndex(
currFiles,
([filename, _]) => filename === payload.file
);
if (idx === -1) {
return [[payload.file, payload.data], ...currFiles];
}
return [[payload.file, payload.data], ...currFiles];
return bringToFront(currFiles, idx);
});
}

Expand All @@ -56,7 +61,7 @@ const App = ({
return setOpenFiles(payload.data);
}

// Everthing else must be ignored.
// Everything else must be ignored.
default:
return;
}
Expand Down
9 changes: 6 additions & 3 deletions ide/packages/panoptes/src/File.css
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ span.ErrorCount {
border-spacing: 0.15em;
}

/* select TH elements of the MethodCallTable class */
.MethodCallTable th {
text-align: left;
}
Expand All @@ -53,11 +52,15 @@ span.ErrorCount {
vertical-align: middle;
}

.MethodCallTable td.active {
.MethodCallTable td.with-result {
cursor: pointer;
}

.MethodCallTable td.with-result.active {
border: 2px solid var(--vscode-focusBorder);
border-radius: 3px;
}

.MethodCallTable td:hover {
.MethodCallTable td.with-result:hover {
box-shadow: 10px 10px 5px var(--vscode-dropdown-background);
}
8 changes: 3 additions & 5 deletions ide/packages/panoptes/src/File.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ const MethodLookupTable = ({ lookup }: { lookup: MethodLookupIdx }) => {
{_.map(step.traitPredicates, (queryIdx, idx) => (
<td
key={idx}
className={classNames({
className={classNames("with-result", {
active: queryIdx === clickedObligation?.[0],
})}
onMouseEnter={onTDHover(queryIdx)}
Expand All @@ -231,9 +231,7 @@ const MethodLookupTable = ({ lookup }: { lookup: MethodLookupIdx }) => {
);
};

// NOTE: don't access the expression obligations directly, use the BodyInfo
// to get the obligations that are currently visible.
const InExpr = ({ idx }: { idx: ExprIdx }) => {
const Expr = ({ idx }: { idx: ExprIdx }) => {
useSignals();

const bodyInfo = useContext(BodyInfoContext)!;
Expand Down Expand Up @@ -316,7 +314,7 @@ const ObligationBody = ({ bodyInfo }: { bodyInfo: BodyInfo }) => {
info={header}
startOpen={openChildren}
Children={() =>
_.map(bodyInfo.exprs, (i, idx) => <InExpr idx={i} key={idx} />)
_.map(bodyInfo.exprs, (i, idx) => <Expr idx={i} key={idx} />)
}
/>
</BodyInfoContext.Provider>
Expand Down
4 changes: 4 additions & 0 deletions ide/packages/panoptes/src/utilities/func.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ export function obligationCardId(file: Filename, hash: ObligationHash) {
return `obl--${name}-${hash}`;
}

export function bringToFront<T>(data: T[], index: number): T[] {
return [data[index], ...data.slice(0, index), ...data.slice(index + 1)];
}

export function errorCardId(
file: Filename,
bodyIdx: number,
Expand Down

0 comments on commit b59d23f

Please sign in to comment.