Nothing compiler/interpreter, Part 2

Question

Part 1

I've followed some of the suggestions:

• Adding newlines to error messages
• Implementing options (I decided to use boost instead of getopt)
• Using strerror for fstream failure
• Allow multiple files to be compiled (to produce one binary - I wasn't sure if the suggestion meant one binary for each file)

Things I did not implement yet:

• Localization
• Reading code from standard input instead of files
• Allowing the user to choose which architecture and platform the binary should be for (trivial)

Things I'm looking for in the review:

• Readability. Especially on the program structure and comments

• Whether or not it is intuitive for somebody who's used to a compiler like GCC. For example, I added --silent as a suggestion from the previous question, but it's not an option that I'm too familiar with. Although it DOES save people the trouble of piping strerr to /dev/null/

• Should the error for chmod be handled at the caller or inside set_executable_permissions?

Example run:

% ./nothing
nothing: fatal error: no input files
compilation terminated.
% ./nothing --help
usage: ./nothing [OPTIONS] <input-file>...
Allowed options:
  --help                            produce help message
  --silent                          suppress error messages
  -o [ --output-file ] arg (=a.out) output file
% ./nothing asdf
nothing: error: asdf: No such file or directory
nothing: fatal error: no input files
compilation terminated.
% ./nothing --silent asdf
% touch empty.not
% ./nothing -o my_program empty.not
% objdump -d ./my_program

./my_program:     file format elf64-x86-64

Disassembly of section .text:

0000000008048000 <.text>:
 8048000:   bb 00 00 00 00          mov    $0x0,%ebx
 8048005:   b8 01 00 00 00          mov    $0x1,%eax
 804800a:   cd 80                   int    $0x80

And the code dump:

/*
 * A 'Nothing' compiler
 * <http://www.turtle.dds.nl/nothing/>
 * September 2010, RoPe Development Inc.
 *
 * Author: authorname
 */
#include <boost/program_options.hpp>
#include <elfio/elfio.hpp>

#include <iostream>
#include <exception>
#include <fstream>
#include <cerrno>
#include <cstdlib> // EXIT_FAILURE
#include <cstring> // std::strerror
#include <string>  // std::char_traits<char>::eof

#include <sys/stat.h> // chmod

/* Both stat and chmod will return 0 on success or -1 on failure and set
 * errno.
 */
int set_executable_permissions(std::string filename)
{
    int rval = 0;
    struct stat st;

    if (stat (filename.c_str(), &st) >= 0)
    {
        /* This will save the user the trouble of running chmod +x on the
         * resulting executable. S_IX* refer to "executable" for "others",
         * "group" and "user" respectively. Refer to the man pages for more
         * information.
         */
        rval = chmod (filename.c_str(), st.st_mode | S_IXOTH | S_IXGRP | S_IXUSR);
    }
    return rval;
}

/* Using the ELFIO library, we will create an ELF executable that
 * consists of the following (equivalent) assembly program:
 *
 * .section text
 *
 *     global _start
 *
 * _start:
 *     ; syscall for exit
 *     mov ebx, 0
 *     mov eax, 1
 *     int 0x80
 *
 * To quote the documentation "3.4 ELF File Writer",
 *
 * "The executable will be created and run on i386 Linux OS platform. It
 * is supposed to run well on both 32 and 64 - bit Linux platforms. The
 * file will be created without invoking the compiler or assembler tools
 * in the usual way (i.e.  translating high level source code that makes
 * use of the standard library functions). Instead, using the ELFIO
 * writer, all the necessary sections and segments of the file will be
 * created and filled explicitly, each, with its appropriate data. The
 * physical file would then be created by the ELFIO library."
 */
void create_executable(std::string filename)
{
    using namespace ELFIO;

    elfio writer;

    /* Initialize empty 'elfio' object. This should be done as the first
     * step when creating a new 'elfio' object as other API is relying
     * on parameters provided – ELF file 32-bits/64-bits and little/big
     * endianness
     */
    writer.create( ELFCLASS64, ELFDATA2LSB );

    /* Other attributes of the file. Linux OS loader does not require
     * full set of the attributes, but they are provided when a regular
     * linker used for creation of ELF files
     */
    writer.set_os_abi( ELFOSABI_LINUX );
    writer.set_type( ET_EXEC );
    writer.set_machine( EM_X86_64 );

    // Create code section
    section* text_sec = writer.sections.add( ".text" );

    /* Set section’s attributes. Section type, flags and alignment have
     * a big significance and controls how this section is treated by a
     * linker or OS loader 
     */
    text_sec->set_type( SHT_PROGBITS );
    text_sec->set_flags( SHF_ALLOC | SHF_EXECINSTR );
    text_sec->set_addr_align( 0x10 );

    // Add data into it
    char text[] = { 
        '\xbb', '\x00', '\x00', '\x00', '\x00',   // mov ebx, 0
        '\xB8', '\x01', '\x00', '\x00', '\x00',   // mov eax, 1           
        '\xCD', '\x80'                            // int 0x80             
    };
    text_sec->set_data( text, sizeof( text ) );

    // Create a loadable segment
    segment* text_seg = writer.segments.add();
    text_seg->set_type( PT_LOAD );
    text_seg->set_virtual_address( 0x08048000 );
    text_seg->set_physical_address( 0x08048000 );
    text_seg->set_flags( PF_X | PF_R );
    text_seg->set_align( 0x1000 );

    // Add code section into program segment
    text_seg->add_section_index( text_sec->get_index(), text_sec->get_addr_align() );

    // Setup entry point
    writer.set_entry( 0x08048000 );

    // Create ELF file
    writer.save( filename );

    if (set_executable_permissions( filename ) == -1)
    {
        std::cerr << "nothing: error: " << std::strerror(errno) << "\n";
        std::cerr << "note: you may have to use chmod +x\n";
    }
}

int main(int argc, char* argv[])
{
    namespace po = boost::program_options;

    try
    {
        bool silent = false;
        if (argc < 2)
        {
            std::cerr << "nothing: fatal error: no input files\n";
            std::cerr << "compilation terminated.\n";
            return EXIT_FAILURE;
        }

        /* The existence of an input-file option may be confusing, so we
         * still allow it to be used but hide it from the help prompt.
         */
        po::options_description visible_options("Allowed options");
        visible_options.add_options()
            ("help", "produce help message")
            ("silent", "suppress error messages")
            ("output-file,o", po::value<std::string>()->default_value("a.out"),
             "output file")
            ;

        po::options_description hidden_options("Hidden options");
        hidden_options.add_options()
            ("input-file", po::value<std::vector<std::string>>(), "input file")
            ;

        po::options_description command_line_options;
        command_line_options.add(visible_options).add(hidden_options);

        po::positional_options_description positional_options;

        // The second parameter specifies the "max count". -1 means
        // unlimited
        positional_options.add("input-file", -1);

        po::variables_map variables_map;
        po::store(po::command_line_parser(argc, argv).
                options(command_line_options).positional(positional_options).run(),
                variables_map);
        po::notify(variables_map);

        if (variables_map.count("help"))
        {
            std::cout << "usage: ./nothing [OPTIONS] <input-file>...\n";
            std::cout << visible_options;
            std::cout.flush();
            return EXIT_SUCCESS;
        }

        if (variables_map.count("silent"))
        {
            silent = true;
        }

        auto input_files = variables_map["input-file"].as<std::vector<std::string>>();

        for (auto&& input_file : input_files)
        {
            std::ifstream program(input_file);

            if (!program)
            {
                if (!silent)
                {
                    /* "errno uses thread-local storage on modern operating systems.
                     * However, there's no guarantee that the fstream functions will
                     * not clobber errno after an errno occurs. The underlying
                     * functions may not set errno at all (direct system calls on
                     * Linux, or Win32). This doesn't work on many real world
                     * implementations."
                     * - strcat,
                     *   <https://stackoverflow.com/questions/17337602/how-to-get-error-message-when-ifstream-open-fails/17338934#comment33958980_17338934>
                     */
                    std::cerr << "nothing: error: " << argv[1] << ": " << std::strerror(errno) << "\n";
                    std::cerr << "nothing: fatal error: no input files\n";
                    std::cerr << "compilation terminated.\n";        
                    return EXIT_FAILURE;
                }
            }

            if (program.peek() != std::char_traits<char>::eof())
            {
                if (!silent)
                {
                    std::cerr << "nothing: fatal error: program is not empty\n";
                    std::cerr << "compilation terminated.\n";
                }
                return EXIT_FAILURE;
            }
        }

        create_executable(variables_map["output-file"].as<std::string>());
    } catch(std::exception& e) {
        std::cerr << "nothing: error: " << e.what() << "\n";
        return EXIT_FAILURE;
    }
}

Answer 1

This seems like a pretty solid solution. I have mainly minor comments.

Comments

The purpose of comments is to explain logic to readers of your code. To that end, try to avoid comments that are trivial. Like:

#include <cstdlib> // EXIT_FAILURE
#include <cstring> // std::strerror
#include <string>  // std::char_traits<char>::eof

#include <sys/stat.h> // chmod

Those comments really achieve nothing for you. Besides, what you need <string> for is std::string, so besides providing no value they're actually misleading.

Similarly, you have this huge block comment explaining about errno using thread-local storage - but you have no threads, so I don't see how that comment is remotely relevant. Just print the error.

Setting Permissions

stat() returns 0 on success, -1 on failure, so checking for >= 0 is weird. Also you're taking the filename by value, which triggers an unnecssary copy. Instead you could do:

int set_executable_permissions(const char* filename)
{
    struct stat st;
    if (stat(filename, &st) == 0) {
        return chmod(filename, st.st_mode | ... );
    }
    else {
        return -1;
    }
}

As-is, if stat() fails, you're returning 0, which seems pretty misleading! Also, people know what chmod is, so you can just do st.st_mode | 0111.

main()

I'd move the whole body into a separate function just so you can remove one layer of indentation:

try {
    return run_compiler(argc, argv);
}
catch (std::exception& e) {
    std::cerr << "nothing: error: " << e.what() << "\n";
    return EXIT_FAILURE;
}

Checking file emptiness

Your check:

program.peek() != std::char_traits<char>::eof()

Preferred:

program.peek() != std::ifstream::traits_type::eof();

Use the traits_type from the stream you're using. It's clearer and doesn't lead to the question of where char_traits comes from.

Also, if the program doesn't exist but silent is false, we still want to fail. So the flow should probably be:

if (!program) {
    if (!silent) {
        std::cerr << ...;
    }
    return EXIT_FAILURE;
}
else if (program.peek() != ... ) {
    if (!silent) {
        std::cerr << ...;
    }
    return EXIT_FAILURE;
}

Furthermore, this sort of calls for a separate function:

bool is_empty_file(std::string const& input_file)
{
    ...
}

that you could just call on each of the files:

if (!std::all_of(input_files.begin(), input_files.end(), is_empty_file))
{
    return EXIT_FAILURE;
}