Skip to content

Commit

Permalink
fix: include sources referenced but not embedded by an object file
Browse files Browse the repository at this point in the history
  • Loading branch information
vaind committed Feb 26, 2023
1 parent 872bd77 commit 07409ca
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 29 deletions.
6 changes: 1 addition & 5 deletions src/commands/debug_files/bundle_sources.rs
Expand Up @@ -87,10 +87,6 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {

for (index, object) in archive.get().objects().enumerate() {
let object = object?;
if object.has_sources() {
eprintln!("skipped {orig_path} (no source info)");
continue;
}

let mut out = output_path.unwrap_or(parent_path).join(filename);
match index {
Expand All @@ -107,7 +103,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
let written = writer.write_object_with_filter(
&object,
&filename.to_string_lossy(),
|file, _source_descriptor| filter_bad_sources(file),
filter_bad_sources,
)?;

if !written {
Expand Down
17 changes: 15 additions & 2 deletions src/commands/debug_files/print_sources.rs
@@ -1,3 +1,4 @@
use std::fmt;
use std::path::Path;

use anyhow::Result;
Expand Down Expand Up @@ -67,9 +68,15 @@ fn print_object_sources(object: &Object) -> Result<()> {
let abs_path = file.abs_path_str();
println!(" {}", &abs_path);
let source = debug_session.source_by_path(abs_path.as_str())?;
match source.as_ref().and_then(|sd| sd.contents()) {
match source {
Some(source) => {
println!(" Embedded, {} bytes", source.len());
print_file_descriptor_detail(
"Embedded",
source.contents().map(|v| format!("{} bytes", v.len())),
);
print_file_descriptor_detail("Url", source.url());
print_file_descriptor_detail("DebugId", source.debug_id());
print_file_descriptor_detail("SourceMap Url", source.source_mapping_url());
}
None => {
if Path::new(&abs_path).exists() {
Expand All @@ -83,3 +90,9 @@ fn print_object_sources(object: &Object) -> Result<()> {
}
Ok(())
}

fn print_file_descriptor_detail<T: fmt::Display>(name: &str, value: Option<T>) {
if let Some(value) = value {
println!(" {name}: {value}");
}
}
35 changes: 18 additions & 17 deletions src/utils/dif_upload.rs
Expand Up @@ -25,7 +25,7 @@ use sha1_smol::Digest;
use symbolic::common::{AsSelf, ByteView, DebugId, SelfCell, Uuid};
use symbolic::debuginfo::macho::{BcSymbolMap, UuidMapping};
use symbolic::debuginfo::pe::PeObject;
use symbolic::debuginfo::sourcebundle::SourceBundleWriter;
use symbolic::debuginfo::sourcebundle::{SourceBundleWriter, SourceFileDescriptor};
use symbolic::debuginfo::{Archive, FileEntry, FileFormat, Object};
use symbolic::il2cpp::ObjectLineMapping;
use walkdir::WalkDir;
Expand Down Expand Up @@ -1104,16 +1104,26 @@ fn extract_embedded_ppdb<'a>(pe: &PeObject, pe_name: &str) -> Result<Option<DifM
}

/// Default filter function to skip over bad sources we do not want to include.
pub fn filter_bad_sources(entry: &FileEntry) -> bool {
pub fn filter_bad_sources(
entry: &FileEntry,
embedded_source: &Option<SourceFileDescriptor>,
) -> bool {
let max_size = Config::current().get_max_dif_item_size();
let path = &entry.abs_path_str();

if entry.name_str().ends_with(".pch") {
// always ignore pch files
// Ignore pch files.
if path.ends_with(".pch") {
return false;
} else if let Ok(meta) = fs::metadata(path) {
}

// Ignore files embedded in the object itself.
if embedded_source.is_some() {
return false;
}

// Ignore files larger than limit (defaults to `DEFAULT_MAX_DIF_ITEM_SIZE`).
if let Ok(meta) = fs::metadata(path) {
let item_size = meta.len();
// ignore files larger than limit (defaults to 1MB)
if item_size > max_size {
warn!(
"Source exceeded maximum item size limit ({}). {}",
Expand Down Expand Up @@ -1156,12 +1166,6 @@ fn create_source_bundles<'a>(
Some(object) => object,
None => continue,
};
if object.has_sources() {
// Do not create standalone source bundles if the original object already contains
// source code. This would just store duplicate information in Sentry.
debug!("skipping {} because it already embeds sources", name);
continue;
}

let temp_file = TempFile::create()?;
let mut writer = SourceBundleWriter::start(BufWriter::new(temp_file.open()?))?;
Expand All @@ -1170,11 +1174,8 @@ fn create_source_bundles<'a>(
// Resolve source files from the object and write their contents into the archive. Skip to
// upload this bundle if no source could be written. This can happen if there is no file or
// line information in the object file, or if none of the files could be resolved.
let written = writer.write_object_with_filter(
object,
dif.file_name(),
|file, _source_descriptor| filter_bad_sources(file),
)?;
let written =
writer.write_object_with_filter(object, dif.file_name(), filter_bad_sources)?;
if !written {
debug!("No sources found for {}", name);
continue;
Expand Down
Expand Up @@ -4,14 +4,14 @@ $ sentry-cli debug-files print-sources tests/integration/_fixtures/Sentry.Sample
pe 623535c7-c0ea-4dee-b99b-4669e99a7ecc-a878d1fa has no sources.
portablepdb 623535c7-c0ea-4dee-b99b-4669e99a7ecc-a878d1fa references sources:
C:/dev/sentry-dotnet/samples/Sentry.Samples.Console.Basic/Program.cs
Embedded, 204 bytes
Embedded: 204 bytes
C:/dev/sentry-dotnet/samples/Sentry.Samples.Console.Basic/obj/Release/net6.0/Sentry.Samples.Console.Basic.GlobalUsings.g.cs
Embedded, 295 bytes
Embedded: 295 bytes
C:/dev/sentry-dotnet/samples/Sentry.Samples.Console.Basic/obj/Release/net6.0/.NETCoreApp,Version=v6.0.AssemblyAttributes.cs
Embedded, 198 bytes
Embedded: 198 bytes
C:/dev/sentry-dotnet/samples/Sentry.Samples.Console.Basic/obj/Release/net6.0/Sentry.Attributes.cs
Embedded, 610 bytes
Embedded: 610 bytes
C:/dev/sentry-dotnet/samples/Sentry.Samples.Console.Basic/obj/Release/net6.0/Sentry.Samples.Console.Basic.AssemblyInfo.cs
Embedded, 1019 bytes
Embedded: 1019 bytes

```

0 comments on commit 07409ca

Please sign in to comment.