From 5e14dc2861d211f1802697670f178551f20952d1 Mon Sep 17 00:00:00 2001 From: Jonathan Foote Date: Thu, 7 Dec 2017 09:55:58 -0500 Subject: [PATCH 1/2] First cut at specializing data decls/defns - Adds a new datum to `Data` that determines whether the corresponding ELF section should have the writeable flag set (`writeable`) - Adds corresponding changes to `Prop`; I think this obeys the warning [here](http://github.com/jfoote/faerie/blob/master/src/artifact.rs#L37) since the change does not reorder the fields - Assumes `Data` should not be mergeable and `CString` should be mergeable - Does not address the alignment comment on https://github.com/m4b/faerie/issues/17 - I wasn't quite sure what you had in mind here - The code does not assert/ensure the defined string is null-terminated --- src/artifact.rs | 13 +++++++++---- src/bin/main.rs | 4 ++-- src/elf.rs | 28 +++++++++++++++++++++++++--- src/mach.rs | 1 + 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/artifact.rs b/src/artifact.rs index d45a675..9c88ba0 100644 --- a/src/artifact.rs +++ b/src/artifact.rs @@ -49,6 +49,8 @@ pub enum ArtifactError { pub struct Prop { pub global: bool, pub function: bool, + pub writeable: bool, + pub cstring: bool, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] @@ -69,8 +71,10 @@ pub enum Decl { DataImport, /// A function defined in this artifact Function { global: bool }, - /// An data object defined in this artifact - Data { global: bool }, + /// A data object defined in this artifact + Data { global: bool, writeable: bool }, + /// A null-terminated string object defined in this artifact + CString { global: bool } } impl Decl { @@ -262,8 +266,9 @@ impl Artifact { match self.declarations.get(&decl_name) { Some(ref stype) => { let prop = match *stype { - &Decl::Data { global } => Prop { global, function: false }, - &Decl::Function { global } => Prop { global, function: true }, + &Decl::CString { global } => Prop { global, function: false, writeable: false, cstring: true }, + &Decl::Data { global, writeable } => Prop { global, function: false, writeable, cstring: false }, + &Decl::Function { global } => Prop { global, function: true, writeable: false, cstring: false}, _ if stype.is_import() => return Err(ArtifactError::ImportDefined(name.as_ref().to_string()).into()), _ => unimplemented!("New Decl variant added but not covered in define method"), }; diff --git a/src/bin/main.rs b/src/bin/main.rs index af1712b..75bf90f 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -62,7 +62,7 @@ fn run (args: Args) -> Result<(), Error> { [ ("deadbeef", Decl::Function { global: false }), ("main", Decl::Function { global: true }), - ("str.1", Decl::Data { global: false }), + ("str.1", Decl::CString { global: false }), ("DEADBEEF", Decl::DataImport), ("printf", Decl::FunctionImport), ].into_iter().cloned() @@ -144,7 +144,7 @@ fn deadbeef (args: Args) -> Result<(), Error> { // FIXME: need to state this isn't a string, but some linkers don't seem to care \o/ // gold complains though: // ld.gold: warning: deadbeef.o: last entry in mergeable string section '.data.DEADBEEF' not null terminated - obj.declare("DEADBEEF", Decl::Data { global: true })?; + obj.declare("DEADBEEF", Decl::Data { global: true, writeable: false })?; obj.define("DEADBEEF", [0xef, 0xbe, 0xad, 0xde].to_vec())?; if args.mach { obj.write::(file)?; diff --git a/src/elf.rs b/src/elf.rs index 78a8b6b..cf2ffe1 100644 --- a/src/elf.rs +++ b/src/elf.rs @@ -137,6 +137,7 @@ impl SymbolBuilder { /// The kind of section this can be; used in [SectionBuilder](struct.SectionBuilder.html) pub enum SectionType { Bits, + Data, String, StrTab, SymTab, @@ -174,6 +175,11 @@ impl SectionBuilder { pub fn alloc(mut self) -> Self { self.alloc = true; self } + /// Make this section writeable + pub fn writeable(mut self, writeable:bool) -> Self { + self.write = writeable; self + } + /// Set the byte offset of this section's name in the corresponding strtab pub fn name_offset(mut self, name_offset: usize) -> Self { self.name_offset = name_offset; self @@ -208,6 +214,10 @@ impl SectionBuilder { shdr.sh_type = SHT_PROGBITS; shdr.sh_flags |= (SHF_MERGE | SHF_STRINGS) as u64; }, + SectionType::Data => { + shdr.sh_addralign = if self.exec { 0x10 } else if self.write { 0x8 } else { 1 }; + shdr.sh_type = SHT_PROGBITS; + } SectionType::StrTab => { shdr.sh_addralign = 0x1; shdr.sh_type = SHT_STRTAB; @@ -411,11 +421,22 @@ impl<'a> Elf<'a> { self.symbols.insert(idx, symbol); self.section_symbols.insert(idx, section_symbol); // FIXME: probably add padding alignment + let mut section = { + let stype = + if prop.function { + SectionType::Bits + } else if prop.cstring { + SectionType::String + } else { + SectionType::Data + }; + let tmp = SectionBuilder::new(size as u64) .name_offset(section_offset) - .section_type(if prop.function { SectionType::Bits } else { SectionType::String }) - .alloc(); + .section_type(stype) + .alloc().writeable(prop.writeable); + // FIXME: I don't like this at all; can make exec() take bool but doesn't match other section properties if prop.function { tmp.exec().create(&self.ctx) } else { tmp.create(&self.ctx) } }; @@ -454,13 +475,14 @@ impl<'a> Elf<'a> { // although we're not in the worst company here: https://github.com/ocaml/ocaml/pull/1330 Decl::Function {..} => (reloc::R_X86_64_PLT32, -4), Decl::Data {..} => (reloc::R_X86_64_PC32, 0), + Decl::CString {..} => (reloc::R_X86_64_PC32, 0), Decl::FunctionImport => (reloc::R_X86_64_PLT32, -4), Decl::DataImport => (reloc::R_X86_64_GOTPCREL, -4), } }; let sym_idx = match *to_type { - Decl::Function {..} | Decl::Data {..} => to_idx + 2, + Decl::Function {..} | Decl::Data {..} | Decl::CString {..} => to_idx + 2, // +2 for NOTYPE and FILE symbols Decl::FunctionImport | Decl::DataImport => to_idx + self.section_symbols.len(), // + section_symbols.len() because this is where the import symbols begin diff --git a/src/mach.rs b/src/mach.rs index 17c5b1d..959510d 100644 --- a/src/mach.rs +++ b/src/mach.rs @@ -545,6 +545,7 @@ fn build_relocations(artifact: &Artifact, symtab: &SymbolTable) -> Relocations { let reloc = match link.to.decl { &Decl::Function {..} => X86_64_RELOC_BRANCH, &Decl::Data {..} => X86_64_RELOC_SIGNED, + &Decl::CString {..} => X86_64_RELOC_SIGNED, &Decl::FunctionImport => X86_64_RELOC_BRANCH, &Decl::DataImport => X86_64_RELOC_GOT_LOAD, }; From 44a1b44be02b15e40055042caccd5c88e98d85bc Mon Sep 17 00:00:00 2001 From: Jonathan Foote Date: Thu, 7 Dec 2017 10:16:34 -0500 Subject: [PATCH 2/2] First cut at specializing data decls/defns This is a version of https://github.com/m4b/faerie/pull/22 for our private repo. Salient notes from original follow: - Adds a new datum to `Data` that determines whether the corresponding ELF section should have the writeable flag set (`writeable`) - Adds corresponding changes to `Prop`; I think this obeys the warning [here](http://github.com/jfoote/faerie/blob/master/src/artifact.rs#L37) since the change does not reorder the fields - Assumes `Data` should not be mergeable and `CString` should be mergeable - Does not address the alignment comment on https://github.com/m4b/faerie/issues/17 - The code does not assert/ensure the defined string is null-terminated --- src/artifact.rs | 13 +++++++++---- src/bin/main.rs | 4 ++-- src/elf.rs | 28 +++++++++++++++++++++++++--- src/mach.rs | 1 + 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/artifact.rs b/src/artifact.rs index d45a675..9c88ba0 100644 --- a/src/artifact.rs +++ b/src/artifact.rs @@ -49,6 +49,8 @@ pub enum ArtifactError { pub struct Prop { pub global: bool, pub function: bool, + pub writeable: bool, + pub cstring: bool, } #[derive(Debug, PartialEq, Eq, PartialOrd, Ord)] @@ -69,8 +71,10 @@ pub enum Decl { DataImport, /// A function defined in this artifact Function { global: bool }, - /// An data object defined in this artifact - Data { global: bool }, + /// A data object defined in this artifact + Data { global: bool, writeable: bool }, + /// A null-terminated string object defined in this artifact + CString { global: bool } } impl Decl { @@ -262,8 +266,9 @@ impl Artifact { match self.declarations.get(&decl_name) { Some(ref stype) => { let prop = match *stype { - &Decl::Data { global } => Prop { global, function: false }, - &Decl::Function { global } => Prop { global, function: true }, + &Decl::CString { global } => Prop { global, function: false, writeable: false, cstring: true }, + &Decl::Data { global, writeable } => Prop { global, function: false, writeable, cstring: false }, + &Decl::Function { global } => Prop { global, function: true, writeable: false, cstring: false}, _ if stype.is_import() => return Err(ArtifactError::ImportDefined(name.as_ref().to_string()).into()), _ => unimplemented!("New Decl variant added but not covered in define method"), }; diff --git a/src/bin/main.rs b/src/bin/main.rs index af1712b..75bf90f 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -62,7 +62,7 @@ fn run (args: Args) -> Result<(), Error> { [ ("deadbeef", Decl::Function { global: false }), ("main", Decl::Function { global: true }), - ("str.1", Decl::Data { global: false }), + ("str.1", Decl::CString { global: false }), ("DEADBEEF", Decl::DataImport), ("printf", Decl::FunctionImport), ].into_iter().cloned() @@ -144,7 +144,7 @@ fn deadbeef (args: Args) -> Result<(), Error> { // FIXME: need to state this isn't a string, but some linkers don't seem to care \o/ // gold complains though: // ld.gold: warning: deadbeef.o: last entry in mergeable string section '.data.DEADBEEF' not null terminated - obj.declare("DEADBEEF", Decl::Data { global: true })?; + obj.declare("DEADBEEF", Decl::Data { global: true, writeable: false })?; obj.define("DEADBEEF", [0xef, 0xbe, 0xad, 0xde].to_vec())?; if args.mach { obj.write::(file)?; diff --git a/src/elf.rs b/src/elf.rs index 78a8b6b..cf2ffe1 100644 --- a/src/elf.rs +++ b/src/elf.rs @@ -137,6 +137,7 @@ impl SymbolBuilder { /// The kind of section this can be; used in [SectionBuilder](struct.SectionBuilder.html) pub enum SectionType { Bits, + Data, String, StrTab, SymTab, @@ -174,6 +175,11 @@ impl SectionBuilder { pub fn alloc(mut self) -> Self { self.alloc = true; self } + /// Make this section writeable + pub fn writeable(mut self, writeable:bool) -> Self { + self.write = writeable; self + } + /// Set the byte offset of this section's name in the corresponding strtab pub fn name_offset(mut self, name_offset: usize) -> Self { self.name_offset = name_offset; self @@ -208,6 +214,10 @@ impl SectionBuilder { shdr.sh_type = SHT_PROGBITS; shdr.sh_flags |= (SHF_MERGE | SHF_STRINGS) as u64; }, + SectionType::Data => { + shdr.sh_addralign = if self.exec { 0x10 } else if self.write { 0x8 } else { 1 }; + shdr.sh_type = SHT_PROGBITS; + } SectionType::StrTab => { shdr.sh_addralign = 0x1; shdr.sh_type = SHT_STRTAB; @@ -411,11 +421,22 @@ impl<'a> Elf<'a> { self.symbols.insert(idx, symbol); self.section_symbols.insert(idx, section_symbol); // FIXME: probably add padding alignment + let mut section = { + let stype = + if prop.function { + SectionType::Bits + } else if prop.cstring { + SectionType::String + } else { + SectionType::Data + }; + let tmp = SectionBuilder::new(size as u64) .name_offset(section_offset) - .section_type(if prop.function { SectionType::Bits } else { SectionType::String }) - .alloc(); + .section_type(stype) + .alloc().writeable(prop.writeable); + // FIXME: I don't like this at all; can make exec() take bool but doesn't match other section properties if prop.function { tmp.exec().create(&self.ctx) } else { tmp.create(&self.ctx) } }; @@ -454,13 +475,14 @@ impl<'a> Elf<'a> { // although we're not in the worst company here: https://github.com/ocaml/ocaml/pull/1330 Decl::Function {..} => (reloc::R_X86_64_PLT32, -4), Decl::Data {..} => (reloc::R_X86_64_PC32, 0), + Decl::CString {..} => (reloc::R_X86_64_PC32, 0), Decl::FunctionImport => (reloc::R_X86_64_PLT32, -4), Decl::DataImport => (reloc::R_X86_64_GOTPCREL, -4), } }; let sym_idx = match *to_type { - Decl::Function {..} | Decl::Data {..} => to_idx + 2, + Decl::Function {..} | Decl::Data {..} | Decl::CString {..} => to_idx + 2, // +2 for NOTYPE and FILE symbols Decl::FunctionImport | Decl::DataImport => to_idx + self.section_symbols.len(), // + section_symbols.len() because this is where the import symbols begin diff --git a/src/mach.rs b/src/mach.rs index 17c5b1d..959510d 100644 --- a/src/mach.rs +++ b/src/mach.rs @@ -545,6 +545,7 @@ fn build_relocations(artifact: &Artifact, symtab: &SymbolTable) -> Relocations { let reloc = match link.to.decl { &Decl::Function {..} => X86_64_RELOC_BRANCH, &Decl::Data {..} => X86_64_RELOC_SIGNED, + &Decl::CString {..} => X86_64_RELOC_SIGNED, &Decl::FunctionImport => X86_64_RELOC_BRANCH, &Decl::DataImport => X86_64_RELOC_GOT_LOAD, };